diff --git a/tiledb/sm/fragment/fragment_info.cc b/tiledb/sm/fragment/fragment_info.cc index 08aa78dbf31..5f3d7939526 100644 --- a/tiledb/sm/fragment/fragment_info.cc +++ b/tiledb/sm/fragment/fragment_info.cc @@ -450,9 +450,10 @@ Status FragmentInfo::get_mbr_num(uint32_t fid, uint64_t* mbr_num) { return Status::Ok(); } - auto meta = single_fragment_info_vec_[fid].meta(); - meta->loaded_metadata()->load_rtree(enc_key_); - *mbr_num = meta->mbrs().size(); + auto loaded_fragment_metadata = + single_fragment_info_vec_[fid].loaded_fragment_metadata(); + loaded_fragment_metadata->load_rtree(enc_key_); + *mbr_num = loaded_fragment_metadata->rtree().leaves().size(); return Status::Ok(); } @@ -472,9 +473,10 @@ Status FragmentInfo::get_mbr( return LOG_STATUS( Status_FragmentInfoError("Cannot get MBR; Fragment is not sparse")); - auto meta = single_fragment_info_vec_[fid].meta(); - meta->loaded_metadata()->load_rtree(enc_key_); - const auto& mbrs = meta->mbrs(); + auto loaded_fragment_metadata = + single_fragment_info_vec_[fid].loaded_fragment_metadata(); + loaded_fragment_metadata->load_rtree(enc_key_); + const auto& mbrs = loaded_fragment_metadata->rtree().leaves(); if (mid >= mbrs.size()) return LOG_STATUS( @@ -554,9 +556,10 @@ Status FragmentInfo::get_mbr_var_size( return LOG_STATUS( Status_FragmentInfoError("Cannot get MBR; Fragment is not sparse")); - auto meta = single_fragment_info_vec_[fid].meta(); - meta->loaded_metadata()->load_rtree(enc_key_); - const auto& mbrs = meta->mbrs(); + auto loaded_fragment_metadata = + single_fragment_info_vec_[fid].loaded_fragment_metadata(); + loaded_fragment_metadata->load_rtree(enc_key_); + const auto& mbrs = loaded_fragment_metadata->rtree().leaves(); if (mid >= mbrs.size()) return LOG_STATUS( @@ -634,9 +637,10 @@ Status FragmentInfo::get_mbr_var( return LOG_STATUS( Status_FragmentInfoError("Cannot get MBR var; Fragment is not sparse")); - auto meta = single_fragment_info_vec_[fid].meta(); - meta->loaded_metadata()->load_rtree(enc_key_); - const auto& mbrs = meta->mbrs(); + auto loaded_fragment_metadata = + single_fragment_info_vec_[fid].loaded_fragment_metadata(); + loaded_fragment_metadata->load_rtree(enc_key_); + const auto& mbrs = loaded_fragment_metadata->rtree().leaves(); if (mid >= mbrs.size()) return LOG_STATUS( @@ -858,7 +862,7 @@ Status FragmentInfo::load(const ArrayDirectory& array_dir) { } if (preload_rtrees & !meta->dense()) { - meta->loaded_metadata()->load_rtree(enc_key_); + meta->loaded_metadata_shared()->load_rtree(enc_key_); } return Status::Ok(); @@ -886,14 +890,21 @@ Status FragmentInfo::load(const ArrayDirectory& array_dir) { array_schema->domain().expand_to_tiles(&expanded_non_empty_domain); // Push new fragment info - single_fragment_info_vec_.emplace_back(SingleFragmentInfo( + single_fragment_info_vec_.emplace_back( uri, sparse, meta->timestamp_range(), sizes[fid], non_empty_domain, expanded_non_empty_domain, - meta)); + meta, + // This workaround is unfortunately absolutely necessary. With this + // workaround we can ship this change in FragmentInfo independently + // and when we break the circular dependency between FragmentMetadata + // and LoadedFragmentMetadata, we can remove this workaround, + // construct LoadedFragmentMetadata objects independenly for which + // SingleFragmentInfo objects can have exclusive ownership. + meta->loaded_metadata_shared()); } } @@ -1134,7 +1145,14 @@ tuple> FragmentInfo::load( size, non_empty_domain, expanded_non_empty_domain, - meta); + meta, + // This workaround is unfortunately absolutely necessary. With this + // workaround we can ship this change in FragmentInfo independently + // and when we break the circular dependency between FragmentMetadata + // and LoadedFragmentMetadata, we can remove this workaround, + // construct LoadedFragmentMetadata objects independenly for which + // SingleFragmentInfo objects can have exclusive ownership. + meta->loaded_metadata_shared()); return {Status::Ok(), ret}; } diff --git a/tiledb/sm/fragment/fragment_metadata.h b/tiledb/sm/fragment/fragment_metadata.h index 931d6d0099f..c7269c830a3 100644 --- a/tiledb/sm/fragment/fragment_metadata.h +++ b/tiledb/sm/fragment/fragment_metadata.h @@ -340,6 +340,11 @@ class FragmentMetadata { return gt_offsets_; } + /** Returns the memory tracker. */ + inline shared_ptr memory_tracker() { + return memory_tracker_; + } + /** * Initializes the fragment metadata structures. * @@ -827,6 +832,10 @@ class FragmentMetadata { return loaded_metadata_ptr_; } + inline shared_ptr loaded_metadata_shared() { + return loaded_metadata_; + } + private: /* ********************************* */ /* PRIVATE ATTRIBUTES */ diff --git a/tiledb/sm/fragment/single_fragment_info.h b/tiledb/sm/fragment/single_fragment_info.h index 4f98b39e1b9..fd7fffd5a25 100644 --- a/tiledb/sm/fragment/single_fragment_info.h +++ b/tiledb/sm/fragment/single_fragment_info.h @@ -72,7 +72,8 @@ class SingleFragmentInfo { uint64_t fragment_size, const NDRange& non_empty_domain, const NDRange& expanded_non_empty_domain, - shared_ptr meta) + shared_ptr meta, + shared_ptr loaded_metadata) : uri_(uri) , name_(meta->fragment_uri().remove_trailing_slash().last_path_part()) , version_(meta->format_version()) @@ -84,7 +85,8 @@ class SingleFragmentInfo { , non_empty_domain_(non_empty_domain) , expanded_non_empty_domain_(expanded_non_empty_domain) , array_schema_name_(meta->array_schema_name()) - , meta_(meta) { + , meta_(meta) + , loaded_fragment_metadata_(loaded_metadata) { } /** Copy constructor. */ @@ -213,11 +215,16 @@ class SingleFragmentInfo { return meta_; } - /** Accessor to the metadata pointer. */ + /** Accessor to the nonempty domain. */ NDRange& non_empty_domain() { return non_empty_domain_; } + /** Accessor to the loaded metadata pointer. */ + shared_ptr loaded_fragment_metadata() const { + return loaded_fragment_metadata_; + } + private: /** The fragment URI. */ URI uri_; @@ -264,6 +271,9 @@ class SingleFragmentInfo { /** The fragment metadata. **/ shared_ptr meta_; + /** The loaded fragment metadata. **/ + shared_ptr loaded_fragment_metadata_; + /** * Returns a deep copy of this FragmentInfo. * @return New FragmentInfo @@ -282,6 +292,7 @@ class SingleFragmentInfo { clone.expanded_non_empty_domain_ = expanded_non_empty_domain_; clone.array_schema_name_ = array_schema_name_; clone.meta_ = meta_; + clone.loaded_fragment_metadata_ = loaded_fragment_metadata_; return clone; } @@ -299,6 +310,7 @@ class SingleFragmentInfo { std::swap(expanded_non_empty_domain_, info.expanded_non_empty_domain_); std::swap(array_schema_name_, info.array_schema_name_); std::swap(meta_, info.meta_); + std::swap(loaded_fragment_metadata_, info.loaded_fragment_metadata_); } }; diff --git a/tiledb/sm/serialization/fragment_info.cc b/tiledb/sm/serialization/fragment_info.cc index d617fe645f6..12e00c84ac6 100644 --- a/tiledb/sm/serialization/fragment_info.cc +++ b/tiledb/sm/serialization/fragment_info.cc @@ -252,7 +252,14 @@ single_fragment_info_from_capnp( single_frag_info_reader.getFragmentSize(), meta->non_empty_domain(), expanded_non_empty_domain, - meta}; + meta, + // This workaround is unfortunately absolutely necessary. With this + // workaround we can ship this change in FragmentInfo independently + // and when we break the circular dependency between FragmentMetadata + // and LoadedFragmentMetadata, we can remove this workaround, + // construct LoadedFragmentMetadata objects independenly for which + // SingleFragmentInfo objects can have exclusive ownership. + meta->loaded_metadata_shared()}; return {Status::Ok(), single_frag_info}; } @@ -269,7 +276,7 @@ Status single_fragment_info_to_capnp( RETURN_NOT_OK( fragment_metadata_to_capnp(*single_frag_info.meta(), &frag_meta_builder)); rtree_to_capnp( - single_frag_info.meta()->loaded_metadata()->rtree(), &frag_meta_builder); + single_frag_info.loaded_fragment_metadata()->rtree(), &frag_meta_builder); // set fragment size single_frag_info_builder->setFragmentSize(single_frag_info.fragment_size());