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

Jump to bottom

fix: persist sst id #1009

Merged
merged 23 commits into from
Jul 7, 2023
Merged

fix: persist sst id #1009

merged 23 commits into from
Jul 7, 2023

Conversation

baojinri
Copy link
Contributor

@baojinri baojinri commented Jun 21, 2023 •
edited by ShiKaiWi
Loading

Rationale

part of #990

Detailed Changes

  • Abstract a IdAllocator in common_util library
  • Use IdAllocator to alloc sst id
  • Persist IdAllocator's max id to manifest

Test Plan

  • Add new unit test for IdAllocator
  • Manual compatibility test: make ssts generated with old ceresdb-server, and deploy a new ceresdb-server with this changeset. Write some new data into it to make new ssts generated, and check whether the sst id is correct.

@baojinri baojinri changed the title refactor: alloc sst id refactor: persist sst id Jun 25, 2023
@baojinri baojinri marked this pull request as ready for review June 25, 2023 09:21
@baojinri baojinri changed the title refactor: persist sst id fix: persist sst id Jun 25, 2023
@baojinri baojinri marked this pull request as draft June 26, 2023 08:41
@baojinri baojinri marked this pull request as ready for review June 27, 2023 07:45
@baojinri baojinri requested a review from ShiKaiWi June 27, 2023 07:45
baojinri
baojinri commented Jun 28, 2023
View reviewed changes
Cargo.toml Outdated Show resolved Hide resolved
@ShiKaiWi
Copy link
Member

@baojinri Could you execute cargo sort --workspace --check to ensure the dep order of Cargo.toml?

chunshao90
chunshao90 reviewed Jul 3, 2023
View reviewed changes
analytic_engine/src/instance/flush_compaction.rs Outdated Show resolved Hide resolved
analytic_engine/src/table/data.rs Outdated Show resolved Hide resolved
analytic_engine/src/table/data.rs Outdated Show resolved Hide resolved
ShiKaiWi
ShiKaiWi reviewed Jul 5, 2023
View reviewed changes
analytic_engine/src/instance/flush_compaction.rs Outdated Show resolved Hide resolved
analytic_engine/src/instance/flush_compaction.rs Outdated Show resolved Hide resolved
analytic_engine/src/table/data.rs Outdated Show resolved Hide resolved
analytic_engine/src/table/data.rs Outdated Show resolved Hide resolved
analytic_engine/src/table/data.rs Outdated Show resolved Hide resolved
analytic_engine/src/table/data.rs Outdated Show resolved Hide resolved
analytic_engine/src/table_meta_set_impl.rs Outdated Show resolved Hide resolved
analytic_engine/src/table_meta_set_impl.rs Show resolved Hide resolved
analytic_engine/src/table_meta_set_impl.rs Outdated Show resolved Hide resolved
@ShiKaiWi
Copy link
Member

ShiKaiWi commented Jul 5, 2023 •
edited
Loading

@baojinri The description of the PR is too simple. Please make it concrete.

ShiKaiWi
ShiKaiWi reviewed Jul 7, 2023
View reviewed changes
Cargo.toml Outdated Show resolved Hide resolved
analytic_engine/src/table/data.rs Outdated Show resolved Hide resolved
analytic_engine/src/table/data.rs Outdated Show resolved Hide resolved
analytic_engine/src/table/data.rs Outdated Show resolved Hide resolved
analytic_engine/src/table/data.rs Outdated Show resolved Hide resolved
analytic_engine/src/table/data.rs Show resolved Hide resolved
common_util/src/id_allocator.rs Outdated Show resolved Hide resolved
common_util/src/id_allocator.rs Outdated Show resolved Hide resolved
common_util/src/id_allocator.rs Outdated Show resolved Hide resolved
ShiKaiWi
ShiKaiWi reviewed Jul 7, 2023
View reviewed changes
analytic_engine/src/table/data.rs Outdated Show resolved Hide resolved
analytic_engine/src/table/version.rs Outdated Show resolved Hide resolved
common_util/src/id_allocator.rs Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
ShiKaiWi
ShiKaiWi approved these changes Jul 7, 2023
View reviewed changes
Copy link
Member

@ShiKaiWi ShiKaiWi left a comment

Choose a reason for hiding this comment

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

LGTM

@ShiKaiWi ShiKaiWi merged commit ed63767 into apache:main Jul 7, 2023
@ShiKaiWi ShiKaiWi mentioned this pull request Jul 7, 2023
Open
@baojinri baojinri deleted the refactor-alloc-sst-id branch July 7, 2023 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Rachelint Rachelint Rachelint left review comments

@chunshao90 chunshao90 chunshao90 left review comments

@ShiKaiWi ShiKaiWi ShiKaiWi approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@baojinri @ShiKaiWi @chunshao90 @Rachelint