Skip to content

Commit

Permalink
break infinite mutual recursion between Group::load_metadata and Rest…
Browse files Browse the repository at this point in the history
…Client::post_group_metadata_from_rest
  • Loading branch information
robertbindar committed May 10, 2022
1 parent 6e15ec9 commit e871744
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 8 deletions.
4 changes: 2 additions & 2 deletions tiledb/sm/rest/rest_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -929,7 +929,7 @@ Status RestClient::post_group_metadata_from_rest(const URI& uri, Group* group) {

Buffer buff;
RETURN_NOT_OK(serialization::group_metadata_serialize(
group, serialization_type_, &buff));
group, serialization_type_, &buff, false));
// Wrap in a list
BufferList serialized;
RETURN_NOT_OK(serialized.add_buffer(std::move(buff)));
Expand Down Expand Up @@ -970,7 +970,7 @@ Status RestClient::put_group_metadata_to_rest(const URI& uri, Group* group) {

Buffer buff;
RETURN_NOT_OK(serialization::group_metadata_serialize(
group, serialization_type_, &buff));
group, serialization_type_, &buff, true));
// Wrap in a list
BufferList serialized;
RETURN_NOT_OK(serialized.add_buffer(std::move(buff)));
Expand Down
21 changes: 16 additions & 5 deletions tiledb/sm/serialization/group.cc
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,20 @@ namespace serialization {
#ifdef TILEDB_SERIALIZATION

Status group_metadata_to_capnp(
Group* group, capnp::GroupMetadata::Builder* group_metadata_builder) {
Group* group,
capnp::GroupMetadata::Builder* group_metadata_builder,
bool load) {
// Set config
auto config_builder = group_metadata_builder->initConfig();
RETURN_NOT_OK(config_to_capnp(group->config(), &config_builder));

Metadata* metadata;
RETURN_NOT_OK(group->metadata(&metadata));
if (load) {
RETURN_NOT_OK(group->metadata(&metadata));
} else {
metadata = const_cast<Metadata*>(group->metadata());
}

if (metadata->num()) {
auto metadata_builder = group_metadata_builder->initMetadata();
RETURN_NOT_OK(metadata_to_capnp(metadata, &metadata_builder));
Expand Down Expand Up @@ -708,12 +715,16 @@ Status group_create_serialize(
}

Status group_metadata_serialize(
Group* group, SerializationType serialize_type, Buffer* serialized_buffer) {
Group* group,
SerializationType serialize_type,
Buffer* serialized_buffer,
bool load) {
try {
::capnp::MallocMessageBuilder message;
capnp::GroupMetadata::Builder group_metadata_builder =
message.initRoot<capnp::GroupMetadata>();
RETURN_NOT_OK(group_metadata_to_capnp(group, &group_metadata_builder));
RETURN_NOT_OK(
group_metadata_to_capnp(group, &group_metadata_builder, load));

serialized_buffer->reset_size();
serialized_buffer->reset_offset();
Expand Down Expand Up @@ -796,7 +807,7 @@ Status group_create_serialize(const Group*, SerializationType, Buffer*) {
"Cannot serialize; serialization not enabled."));
}

Status group_metadata_serialize(Group*, SerializationType, Buffer*) {
Status group_metadata_serialize(Group*, SerializationType, Buffer*, bool load) {
return LOG_STATUS(Status_SerializationError(
"Cannot serialize; serialization not enabled."));
}
Expand Down
7 changes: 6 additions & 1 deletion tiledb/sm/serialization/group.h
Original file line number Diff line number Diff line change
Expand Up @@ -146,10 +146,15 @@ Status group_create_serialize(
* @param serialize_type format to serialize into Cap'n Proto or JSON
* @param serialized_buffer buffer to store serialized bytes in
* serialize the array URI
* @param load flag signaling whether or not metadata should be fetched from
* storage
* @return Status
*/
Status group_metadata_serialize(
Group* group, SerializationType serialize_type, Buffer* serialized_buffer);
Group* group,
SerializationType serialize_type,
Buffer* serialized_buffer,
bool load);

This comment has been minimized.

Copy link
@Shelnutt2

Shelnutt2 May 10, 2022

Member

The Group object has the metadata_loaded_ bool. Can we use this and handle it internally?

This comment has been minimized.

Copy link
@robertbindar

robertbindar May 11, 2022

Author Contributor

I thought about this but it looks like you'd have to set metadata_loaded_ somewhere before the rest call that brings in the metadata.
I have a question though, why do we serialize and send metadata in the post_group_metadata_from_rest call? I have a feeling that this is how we should handle the problem at the root cause.


} // namespace serialization
} // namespace sm
Expand Down

0 comments on commit e871744

Please sign in to comment.