From 348d12dfa309b51c15fa8ce61ffb21625e56fc07 Mon Sep 17 00:00:00 2001 From: Robert Bindar Date: Wed, 28 Aug 2024 18:56:49 +0300 Subject: [PATCH 01/10] test rtree checks --- tiledb/sm/serialization/array.cc | 2 +- tiledb/sm/serialization/fragment_info.cc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tiledb/sm/serialization/array.cc b/tiledb/sm/serialization/array.cc index ad75d63547a..ee9aa42bc24 100644 --- a/tiledb/sm/serialization/array.cc +++ b/tiledb/sm/serialization/array.cc @@ -327,7 +327,7 @@ void array_from_capnp( throw_if_not_ok( fragment_metadata_from_capnp(schema, frag_meta_reader, meta)); if (client_side) { - meta->loaded_metadata()->set_rtree_loaded(); + // meta->loaded_metadata()->set_rtree_loaded(); } fragment_metadata.emplace_back(meta); } diff --git a/tiledb/sm/serialization/fragment_info.cc b/tiledb/sm/serialization/fragment_info.cc index a26071d38ec..8e6f58bfea3 100644 --- a/tiledb/sm/serialization/fragment_info.cc +++ b/tiledb/sm/serialization/fragment_info.cc @@ -254,7 +254,7 @@ single_fragment_info_from_capnp( expanded_non_empty_domain, meta}; // This is needed so that we don't try to load rtee from disk - single_frag_info.meta()->loaded_metadata()->set_rtree_loaded(); + // single_frag_info.meta()->loaded_metadata()->set_rtree_loaded(); return {Status::Ok(), single_frag_info}; } From 8ff7502d7a058793788863898d4d003538e8b320 Mon Sep 17 00:00:00 2001 From: Robert Bindar Date: Thu, 29 Aug 2024 12:53:10 +0300 Subject: [PATCH 02/10] one more try --- tiledb/sm/serialization/fragment_info.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tiledb/sm/serialization/fragment_info.cc b/tiledb/sm/serialization/fragment_info.cc index 8e6f58bfea3..a26071d38ec 100644 --- a/tiledb/sm/serialization/fragment_info.cc +++ b/tiledb/sm/serialization/fragment_info.cc @@ -254,7 +254,7 @@ single_fragment_info_from_capnp( expanded_non_empty_domain, meta}; // This is needed so that we don't try to load rtee from disk - // single_frag_info.meta()->loaded_metadata()->set_rtree_loaded(); + single_frag_info.meta()->loaded_metadata()->set_rtree_loaded(); return {Status::Ok(), single_frag_info}; } From 9ec92798ba6fee6e10a8aeac6b2362243749bbc5 Mon Sep 17 00:00:00 2001 From: Robert Bindar Date: Thu, 29 Aug 2024 15:36:45 +0300 Subject: [PATCH 03/10] one more try --- tiledb/sm/serialization/array.cc | 6 +++--- tiledb/sm/serialization/fragment_metadata.cc | 2 ++ 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/tiledb/sm/serialization/array.cc b/tiledb/sm/serialization/array.cc index ee9aa42bc24..6dd9c34945c 100644 --- a/tiledb/sm/serialization/array.cc +++ b/tiledb/sm/serialization/array.cc @@ -326,9 +326,9 @@ void array_from_capnp( // pass the right schema to deserialize fragment metadata throw_if_not_ok( fragment_metadata_from_capnp(schema, frag_meta_reader, meta)); - if (client_side) { - // meta->loaded_metadata()->set_rtree_loaded(); - } + // if (client_side) { + // meta->loaded_metadata()->set_rtree_loaded(); + // } fragment_metadata.emplace_back(meta); } array->set_fragment_metadata(std::move(fragment_metadata)); diff --git a/tiledb/sm/serialization/fragment_metadata.cc b/tiledb/sm/serialization/fragment_metadata.cc index bd4ebb3e372..09bb6b31d7d 100644 --- a/tiledb/sm/serialization/fragment_metadata.cc +++ b/tiledb/sm/serialization/fragment_metadata.cc @@ -388,6 +388,8 @@ Status fragment_metadata_from_capnp( // deserialize it as well in that way. frag_meta->loaded_metadata()->rtree().deserialize( deserializer, &domain, constants::format_version); + + frag_meta->loaded_metadata()->set_rtree_loaded(); } // It's important to do this here as init_domain depends on some fields From f4d6a733b25f8958312a033f0aa356a5a5447a2d Mon Sep 17 00:00:00 2001 From: Robert Bindar Date: Thu, 29 Aug 2024 17:42:32 +0300 Subject: [PATCH 04/10] last check --- tiledb/sm/serialization/fragment_metadata.cc | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/tiledb/sm/serialization/fragment_metadata.cc b/tiledb/sm/serialization/fragment_metadata.cc index 09bb6b31d7d..dee27d2bafc 100644 --- a/tiledb/sm/serialization/fragment_metadata.cc +++ b/tiledb/sm/serialization/fragment_metadata.cc @@ -694,15 +694,16 @@ Status fragment_metadata_to_capnp( // TODO: Can this be done better? Does this make a lot of copies? SizeComputationSerializer size_computation_serializer; frag_meta.loaded_metadata()->rtree().serialize(size_computation_serializer); + if (size_computation_serializer.size() != 0) { + std::vector buff(size_computation_serializer.size()); + Serializer serializer(buff.data(), buff.size()); + frag_meta.loaded_metadata()->rtree().serialize(serializer); - std::vector buff(size_computation_serializer.size()); - Serializer serializer(buff.data(), buff.size()); - frag_meta.loaded_metadata()->rtree().serialize(serializer); - - auto vec = kj::Vector(); - vec.addAll( - kj::ArrayPtr(static_cast(buff.data()), buff.size())); - frag_meta_builder->setRtree(vec.asPtr()); + auto vec = kj::Vector(); + vec.addAll( + kj::ArrayPtr(static_cast(buff.data()), buff.size())); + frag_meta_builder->setRtree(vec.asPtr()); + } auto gt_offsets_builder = frag_meta_builder->initGtOffsets(); generic_tile_offsets_to_capnp( From 27c1957ea91bc32f5793534ea30404437813e861 Mon Sep 17 00:00:00 2001 From: Robert Bindar Date: Thu, 29 Aug 2024 18:58:30 +0300 Subject: [PATCH 05/10] retry --- tiledb/sm/serialization/array.cc | 6 +++--- tiledb/sm/serialization/fragment_metadata.cc | 22 ++++++++++---------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/tiledb/sm/serialization/array.cc b/tiledb/sm/serialization/array.cc index 6dd9c34945c..ad75d63547a 100644 --- a/tiledb/sm/serialization/array.cc +++ b/tiledb/sm/serialization/array.cc @@ -326,9 +326,9 @@ void array_from_capnp( // pass the right schema to deserialize fragment metadata throw_if_not_ok( fragment_metadata_from_capnp(schema, frag_meta_reader, meta)); - // if (client_side) { - // meta->loaded_metadata()->set_rtree_loaded(); - // } + if (client_side) { + meta->loaded_metadata()->set_rtree_loaded(); + } fragment_metadata.emplace_back(meta); } array->set_fragment_metadata(std::move(fragment_metadata)); diff --git a/tiledb/sm/serialization/fragment_metadata.cc b/tiledb/sm/serialization/fragment_metadata.cc index dee27d2bafc..d6402d25ed2 100644 --- a/tiledb/sm/serialization/fragment_metadata.cc +++ b/tiledb/sm/serialization/fragment_metadata.cc @@ -389,7 +389,7 @@ Status fragment_metadata_from_capnp( frag_meta->loaded_metadata()->rtree().deserialize( deserializer, &domain, constants::format_version); - frag_meta->loaded_metadata()->set_rtree_loaded(); + // frag_meta->loaded_metadata()->set_rtree_loaded(); } // It's important to do this here as init_domain depends on some fields @@ -694,16 +694,16 @@ Status fragment_metadata_to_capnp( // TODO: Can this be done better? Does this make a lot of copies? SizeComputationSerializer size_computation_serializer; frag_meta.loaded_metadata()->rtree().serialize(size_computation_serializer); - if (size_computation_serializer.size() != 0) { - std::vector buff(size_computation_serializer.size()); - Serializer serializer(buff.data(), buff.size()); - frag_meta.loaded_metadata()->rtree().serialize(serializer); - - auto vec = kj::Vector(); - vec.addAll( - kj::ArrayPtr(static_cast(buff.data()), buff.size())); - frag_meta_builder->setRtree(vec.asPtr()); - } + // if (size_computation_serializer.size() != 0) { + std::vector buff(size_computation_serializer.size()); + Serializer serializer(buff.data(), buff.size()); + frag_meta.loaded_metadata()->rtree().serialize(serializer); + + auto vec = kj::Vector(); + vec.addAll( + kj::ArrayPtr(static_cast(buff.data()), buff.size())); + frag_meta_builder->setRtree(vec.asPtr()); + // } auto gt_offsets_builder = frag_meta_builder->initGtOffsets(); generic_tile_offsets_to_capnp( From 4ffb21b8da98bdc0e3b36201a6d0e11242ef99c1 Mon Sep 17 00:00:00 2001 From: Robert Bindar Date: Fri, 30 Aug 2024 12:08:03 +0300 Subject: [PATCH 06/10] try again --- tiledb/sm/serialization/array.cc | 6 ++-- tiledb/sm/serialization/fragment_metadata.cc | 22 ++++++------ tiledb/sm/serialization/query.cc | 35 ++++++++++++++------ 3 files changed, 39 insertions(+), 24 deletions(-) diff --git a/tiledb/sm/serialization/array.cc b/tiledb/sm/serialization/array.cc index ad75d63547a..44fb4a59f13 100644 --- a/tiledb/sm/serialization/array.cc +++ b/tiledb/sm/serialization/array.cc @@ -326,9 +326,9 @@ void array_from_capnp( // pass the right schema to deserialize fragment metadata throw_if_not_ok( fragment_metadata_from_capnp(schema, frag_meta_reader, meta)); - if (client_side) { - meta->loaded_metadata()->set_rtree_loaded(); - } + // if (client_side) { + // meta->loaded_metadata()->set_rtree_loaded(); + // } fragment_metadata.emplace_back(meta); } array->set_fragment_metadata(std::move(fragment_metadata)); diff --git a/tiledb/sm/serialization/fragment_metadata.cc b/tiledb/sm/serialization/fragment_metadata.cc index d6402d25ed2..dee27d2bafc 100644 --- a/tiledb/sm/serialization/fragment_metadata.cc +++ b/tiledb/sm/serialization/fragment_metadata.cc @@ -389,7 +389,7 @@ Status fragment_metadata_from_capnp( frag_meta->loaded_metadata()->rtree().deserialize( deserializer, &domain, constants::format_version); - // frag_meta->loaded_metadata()->set_rtree_loaded(); + frag_meta->loaded_metadata()->set_rtree_loaded(); } // It's important to do this here as init_domain depends on some fields @@ -694,16 +694,16 @@ Status fragment_metadata_to_capnp( // TODO: Can this be done better? Does this make a lot of copies? SizeComputationSerializer size_computation_serializer; frag_meta.loaded_metadata()->rtree().serialize(size_computation_serializer); - // if (size_computation_serializer.size() != 0) { - std::vector buff(size_computation_serializer.size()); - Serializer serializer(buff.data(), buff.size()); - frag_meta.loaded_metadata()->rtree().serialize(serializer); - - auto vec = kj::Vector(); - vec.addAll( - kj::ArrayPtr(static_cast(buff.data()), buff.size())); - frag_meta_builder->setRtree(vec.asPtr()); - // } + if (size_computation_serializer.size() != 0) { + std::vector buff(size_computation_serializer.size()); + Serializer serializer(buff.data(), buff.size()); + frag_meta.loaded_metadata()->rtree().serialize(serializer); + + auto vec = kj::Vector(); + vec.addAll( + kj::ArrayPtr(static_cast(buff.data()), buff.size())); + frag_meta_builder->setRtree(vec.asPtr()); + } auto gt_offsets_builder = frag_meta_builder->initGtOffsets(); generic_tile_offsets_to_capnp( diff --git a/tiledb/sm/serialization/query.cc b/tiledb/sm/serialization/query.cc index 65127afa1d3..45d4b6d8814 100644 --- a/tiledb/sm/serialization/query.cc +++ b/tiledb/sm/serialization/query.cc @@ -621,7 +621,7 @@ Status read_state_from_capnp( const capnp::ReadState::Reader& read_state_reader, Query* query, Reader* reader, - ThreadPool* compute_tp) { + ThreadPool* compute_tp bool client_side) { auto read_state = reader->read_state(); read_state->overflowed_ = read_state_reader.getOverflowed(); @@ -641,7 +641,7 @@ Status read_state_from_capnp( // If the current partition is unsplittable, this means we need to make // sure the tile_overlap for the current is computed because we won't go // to the next partition - read_state->unsplittable_)); + read_state->unsplittable_ && !client_side)); } return Status::Ok(); @@ -669,7 +669,8 @@ Status dense_read_state_from_capnp( const capnp::ReadState::Reader& read_state_reader, Query* query, DenseReader* reader, - ThreadPool* compute_tp) { + ThreadPool* compute_tp, + bool client_side) { auto read_state = reader->read_state(); read_state->overflowed_ = read_state_reader.getOverflowed(); @@ -689,7 +690,7 @@ Status dense_read_state_from_capnp( // If the current partition is unsplittable, this means we need to make // sure the tile_overlap for the current is computed because we won't go // to the next partition - read_state->unsplittable_)); + read_state->unsplittable_ && !client_side)); } return Status::Ok(); @@ -1097,7 +1098,8 @@ Status reader_from_capnp( const capnp::QueryReader::Reader& reader_reader, Query* query, Reader* reader, - ThreadPool* compute_tp) { + ThreadPool* compute_tp, + bool client_side) { auto array = query->array(); // Layout @@ -1113,7 +1115,12 @@ Status reader_from_capnp( // Read state if (reader_reader.hasReadState()) RETURN_NOT_OK(read_state_from_capnp( - array, reader_reader.getReadState(), query, reader, compute_tp)); + array, + reader_reader.getReadState(), + query, + reader, + compute_tp, + client_side)); // Query condition if (reader_reader.hasCondition()) { @@ -1174,7 +1181,8 @@ Status dense_reader_from_capnp( const capnp::QueryReader::Reader& reader_reader, Query* query, DenseReader* reader, - ThreadPool* compute_tp) { + ThreadPool* compute_tp, + bool client_side) { auto array = query->array(); // Layout @@ -1190,7 +1198,12 @@ Status dense_reader_from_capnp( // Read state if (reader_reader.hasReadState()) RETURN_NOT_OK(dense_read_state_from_capnp( - array, reader_reader.getReadState(), query, reader, compute_tp)); + array, + reader_reader.getReadState(), + query, + reader, + compute_tp, + client_side)); // Query condition if (reader_reader.hasCondition()) { @@ -2161,14 +2174,16 @@ Status query_from_capnp( reader_reader, query, dynamic_cast(query->strategy()), - compute_tp)); + compute_tp, + client_side)); } else { auto reader_reader = query_reader.getReader(); RETURN_NOT_OK(reader_from_capnp( reader_reader, query, dynamic_cast(query->strategy()), - compute_tp)); + compute_tp, + client_side)); } } else if (query_type == QueryType::WRITE) { auto writer_reader = query_reader.getWriter(); From b151e6be1c3481444919a434cf75db04caa8ad5d Mon Sep 17 00:00:00 2001 From: Robert Bindar Date: Fri, 30 Aug 2024 12:12:20 +0300 Subject: [PATCH 07/10] try again --- tiledb/sm/serialization/query.cc | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tiledb/sm/serialization/query.cc b/tiledb/sm/serialization/query.cc index 45d4b6d8814..ed4f50c932e 100644 --- a/tiledb/sm/serialization/query.cc +++ b/tiledb/sm/serialization/query.cc @@ -621,7 +621,8 @@ Status read_state_from_capnp( const capnp::ReadState::Reader& read_state_reader, Query* query, Reader* reader, - ThreadPool* compute_tp bool client_side) { + ThreadPool* compute_tp, + bool client_side) { auto read_state = reader->read_state(); read_state->overflowed_ = read_state_reader.getOverflowed(); @@ -2175,7 +2176,7 @@ Status query_from_capnp( query, dynamic_cast(query->strategy()), compute_tp, - client_side)); + context == SerializationContext::CLIENT)); } else { auto reader_reader = query_reader.getReader(); RETURN_NOT_OK(reader_from_capnp( @@ -2183,7 +2184,7 @@ Status query_from_capnp( query, dynamic_cast(query->strategy()), compute_tp, - client_side)); + context == SerializationContext::CLIENT)); } } else if (query_type == QueryType::WRITE) { auto writer_reader = query_reader.getWriter(); From 3b2511560a3155cb5f923555d1f547698cdb511f Mon Sep 17 00:00:00 2001 From: Robert Bindar Date: Mon, 2 Sep 2024 17:19:25 +0300 Subject: [PATCH 08/10] split rtree_to_capnp, call it in fragment_info --- tiledb/sm/serialization/array.cc | 3 -- tiledb/sm/serialization/fragment_info.cc | 2 ++ tiledb/sm/serialization/fragment_metadata.cc | 33 +++++++++++--------- tiledb/sm/serialization/fragment_metadata.h | 9 ++++++ 4 files changed, 30 insertions(+), 17 deletions(-) diff --git a/tiledb/sm/serialization/array.cc b/tiledb/sm/serialization/array.cc index 44fb4a59f13..0bd62be67a7 100644 --- a/tiledb/sm/serialization/array.cc +++ b/tiledb/sm/serialization/array.cc @@ -326,9 +326,6 @@ void array_from_capnp( // pass the right schema to deserialize fragment metadata throw_if_not_ok( fragment_metadata_from_capnp(schema, frag_meta_reader, meta)); - // if (client_side) { - // meta->loaded_metadata()->set_rtree_loaded(); - // } fragment_metadata.emplace_back(meta); } array->set_fragment_metadata(std::move(fragment_metadata)); diff --git a/tiledb/sm/serialization/fragment_info.cc b/tiledb/sm/serialization/fragment_info.cc index a26071d38ec..669a718065a 100644 --- a/tiledb/sm/serialization/fragment_info.cc +++ b/tiledb/sm/serialization/fragment_info.cc @@ -270,6 +270,8 @@ Status single_fragment_info_to_capnp( auto frag_meta_builder = single_frag_info_builder->initMeta(); 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); // set fragment size single_frag_info_builder->setFragmentSize(single_frag_info.fragment_size()); diff --git a/tiledb/sm/serialization/fragment_metadata.cc b/tiledb/sm/serialization/fragment_metadata.cc index dee27d2bafc..ec9d0325a86 100644 --- a/tiledb/sm/serialization/fragment_metadata.cc +++ b/tiledb/sm/serialization/fragment_metadata.cc @@ -537,6 +537,25 @@ void fragment_meta_sizes_offsets_to_capnp( } } } + + rtree_to_capnp(frag_meta.loaded_metadata()->rtree(), frag_meta_builder); +} + +void rtree_to_capnp( + const RTree& rtree, capnp::FragmentMetadata::Builder* frag_meta_builder) { + // TODO: Can this be done better? Does this make a lot of copies? + SizeComputationSerializer size_computation_serializer; + rtree.serialize(size_computation_serializer); + if (size_computation_serializer.size() != 0) { + std::vector buff(size_computation_serializer.size()); + Serializer serializer(buff.data(), buff.size()); + rtree.serialize(serializer); + + auto vec = kj::Vector(); + vec.addAll( + kj::ArrayPtr(static_cast(buff.data()), buff.size())); + frag_meta_builder->setRtree(vec.asPtr()); + } } Status fragment_metadata_to_capnp( @@ -691,20 +710,6 @@ Status fragment_metadata_to_capnp( frag_meta.non_empty_domain(), frag_meta.array_schema()->dim_num())); - // TODO: Can this be done better? Does this make a lot of copies? - SizeComputationSerializer size_computation_serializer; - frag_meta.loaded_metadata()->rtree().serialize(size_computation_serializer); - if (size_computation_serializer.size() != 0) { - std::vector buff(size_computation_serializer.size()); - Serializer serializer(buff.data(), buff.size()); - frag_meta.loaded_metadata()->rtree().serialize(serializer); - - auto vec = kj::Vector(); - vec.addAll( - kj::ArrayPtr(static_cast(buff.data()), buff.size())); - frag_meta_builder->setRtree(vec.asPtr()); - } - auto gt_offsets_builder = frag_meta_builder->initGtOffsets(); generic_tile_offsets_to_capnp( frag_meta.generic_tile_offsets(), gt_offsets_builder); diff --git a/tiledb/sm/serialization/fragment_metadata.h b/tiledb/sm/serialization/fragment_metadata.h index e570bc7fba7..6741c83f01e 100644 --- a/tiledb/sm/serialization/fragment_metadata.h +++ b/tiledb/sm/serialization/fragment_metadata.h @@ -84,6 +84,15 @@ void fragment_meta_sizes_offsets_to_capnp( const FragmentMetadata& frag_meta, capnp::FragmentMetadata::Builder* frag_meta_builder); +/** + * Serializes FragmentMetadata's RTree to Cap'n Proto message + * + * @param rtree RTREE to serialize + * @param frag_meta_builder cap'n proto class + */ +void rtree_to_capnp( + const RTree& rtree, capnp::FragmentMetadata::Builder* frag_meta_builder); + /** * Convert Fragment Metadata to Cap'n Proto message * From 87aa91138205718fec6549a4e3e16aceb41f1801 Mon Sep 17 00:00:00 2001 From: Robert Bindar Date: Tue, 3 Sep 2024 14:06:17 +0300 Subject: [PATCH 09/10] remove redundant set_rtree_loaded --- tiledb/sm/serialization/fragment_info.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/tiledb/sm/serialization/fragment_info.cc b/tiledb/sm/serialization/fragment_info.cc index 669a718065a..d617fe645f6 100644 --- a/tiledb/sm/serialization/fragment_info.cc +++ b/tiledb/sm/serialization/fragment_info.cc @@ -253,8 +253,6 @@ single_fragment_info_from_capnp( meta->non_empty_domain(), expanded_non_empty_domain, meta}; - // This is needed so that we don't try to load rtee from disk - single_frag_info.meta()->loaded_metadata()->set_rtree_loaded(); return {Status::Ok(), single_frag_info}; } From 6a70cba2ad52fda3aaf9fc82b09e7651d9c0a3a0 Mon Sep 17 00:00:00 2001 From: Robert Bindar Date: Tue, 3 Sep 2024 17:44:32 +0300 Subject: [PATCH 10/10] fix incorrect propagation of loaded rtree --- tiledb/sm/serialization/fragment_metadata.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tiledb/sm/serialization/fragment_metadata.cc b/tiledb/sm/serialization/fragment_metadata.cc index ec9d0325a86..7328cb77668 100644 --- a/tiledb/sm/serialization/fragment_metadata.cc +++ b/tiledb/sm/serialization/fragment_metadata.cc @@ -372,6 +372,8 @@ Status fragment_metadata_from_capnp( } frag_meta->last_tile_cell_num() = frag_meta_reader.getLastTileCellNum(); + frag_meta->loaded_metadata()->set_loaded_metadata(loaded_metadata); + if (frag_meta_reader.hasRtree()) { auto data = frag_meta_reader.getRtree(); auto& domain = fragment_array_schema->domain(); @@ -413,8 +415,6 @@ Status fragment_metadata_from_capnp( frag_meta_reader.getGtOffsets(), frag_meta->generic_tile_offsets()); } - frag_meta->loaded_metadata()->set_loaded_metadata(loaded_metadata); - return Status::Ok(); }