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.

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

feat(remote_wal): support persisting per-region unique metadata #2816

Closed
wants to merge 3 commits into from

Conversation

niebayes
Copy link
Contributor

@niebayes niebayes commented Nov 24, 2023

I hereby agree to the terms of the GreptimeDB CLA

What's changed and what's your intention?

Support persisting per-region unique metadata to kv backend through the RegionMetadataManager in the TableMetadataManager.
This feature is currently to support persisting the allocated kafka topic for each region. Meta srv now is able to store per-region topics in the kv backend while handling create table requests. Datanode now is able to retrieve the stored per-region topics while re-opening regions for a table during initializing a region server.

Checklist

  • I have written the necessary rustdoc comments.
  • I have added the necessary unit tests and integration tests.

Refer to a related PR or issue link (optional)

@niebayes niebayes marked this pull request as draft November 24, 2023 10:11
Copy link

codecov bot commented Nov 24, 2023

Codecov Report

Merging #2816 (697b893) into develop (ff8ab67) will decrease coverage by 0.35%.
Report is 4 commits behind head on develop.
The diff coverage is 96.77%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2816      +/-   ##
===========================================
- Coverage    84.75%   84.41%   -0.35%     
===========================================
  Files          730      733       +3     
  Lines       113790   114843    +1053     
===========================================
+ Hits         96445    96947     +502     
- Misses       17345    17896     +551     

@niebayes niebayes marked this pull request as ready for review November 24, 2023 14:07
@niebayes niebayes requested a review from v0y4g3r November 24, 2023 14:07
Comment on lines +112 to +115
fn region_meta_value_decoder(kv: KeyValue) -> Result<((), RegionMetaValue)> {
let value = RegionMetaValue::try_from_raw_value(&kv.value)?;
Ok(((), value))
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like it always allocates a new function.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@WenyXu Moved the function outside.

Comment on lines +31 to +32
// TODO(niebayes): to be removed when `Kafka Remote Wal` is merged.
pub type KafkaTopic = String;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: so what it should be🥲

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be WalTopic.

@niebayes niebayes changed the title feat: support persisting per-region unique metadata feat(remote_wal): support persisting per-region unique metadata Nov 28, 2023
#[derive(Clone)]
pub struct RegionMeta {
region_id: RegionId,
topic: Option<KafkaTopic>,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think wal_topic is more meaningful. If you agree, please rename all places that use "topic" in this file.

Comment on lines +31 to +32
// TODO(niebayes): to be removed when `Kafka Remote Wal` is merged.
pub type KafkaTopic = String;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be WalTopic.

@niebayes
Copy link
Contributor Author

Closed for now and maybe reopened in the future.

@niebayes niebayes closed this Mar 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants