Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor LoadedFragmentMetadata in FragmentInfo #5281

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Refactor LoadedFragmentMetadata in FragmentInfo #5281

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 34 additions & 16 deletions tiledb/sm/fragment/fragment_info.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand All @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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());
}
}

Expand Down Expand Up @@ -1134,7 +1145,14 @@ tuple<Status, optional<SingleFragmentInfo>> 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};
}
Expand Down
9 changes: 9 additions & 0 deletions tiledb/sm/fragment/fragment_metadata.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,11 @@ class FragmentMetadata {
return gt_offsets_;
}

/** Returns the memory tracker. */
inline shared_ptr<MemoryTracker> memory_tracker() {
return memory_tracker_;
}

/**
* Initializes the fragment metadata structures.
*
Expand Down Expand Up @@ -827,6 +832,10 @@ class FragmentMetadata {
return loaded_metadata_ptr_;
}

inline shared_ptr<LoadedFragmentMetadata> loaded_metadata_shared() {
return loaded_metadata_;
}

private:
/* ********************************* */
/* PRIVATE ATTRIBUTES */
Expand Down
18 changes: 15 additions & 3 deletions tiledb/sm/fragment/single_fragment_info.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ class SingleFragmentInfo {
uint64_t fragment_size,
const NDRange& non_empty_domain,
const NDRange& expanded_non_empty_domain,
shared_ptr<FragmentMetadata> meta)
shared_ptr<FragmentMetadata> meta,
shared_ptr<LoadedFragmentMetadata> loaded_metadata)
: uri_(uri)
, name_(meta->fragment_uri().remove_trailing_slash().last_path_part())
, version_(meta->format_version())
Expand All @@ -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. */
Expand Down Expand Up @@ -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<LoadedFragmentMetadata> loaded_fragment_metadata() const {
return loaded_fragment_metadata_;
}

private:
/** The fragment URI. */
URI uri_;
Expand Down Expand Up @@ -264,6 +271,9 @@ class SingleFragmentInfo {
/** The fragment metadata. **/
shared_ptr<FragmentMetadata> meta_;

/** The loaded fragment metadata. **/
shared_ptr<LoadedFragmentMetadata> loaded_fragment_metadata_;

/**
* Returns a deep copy of this FragmentInfo.
* @return New FragmentInfo
Expand All @@ -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;
}

Expand All @@ -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_);
}
};

Expand Down
11 changes: 9 additions & 2 deletions tiledb/sm/serialization/fragment_info.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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};
}
Expand All @@ -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());
Expand Down
Loading