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: place table datas into manifest, update them together #863

Merged
merged 13 commits into from
May 9, 2023

Conversation

Rachelint
Copy link
Contributor

@Rachelint Rachelint commented Apr 26, 2023

Which issue does this PR close?

Closes #
Part of #799

Rationale for this change

Now, we update TableData and store its wal seperately. The order of two operations above is maintained by developer, that will lead to a big bug if developer is not so careful when modifying related codes.

What changes are included in this PR?

  • Place table datas into manifest.
  • Update it both in memory and storage in the same call.

Are there any user-facing changes?

None.

How does this change test

Test by ut.

@Rachelint Rachelint force-pushed the place-table-datas-into-manifest branch from 55dcf8e to 00defb3 Compare April 27, 2023 10:28
@Rachelint Rachelint force-pushed the place-table-datas-into-manifest branch from b488828 to d4e4ac4 Compare April 27, 2023 17:33
@Rachelint Rachelint force-pushed the place-table-datas-into-manifest branch from d4e4ac4 to cb1567c Compare May 4, 2023 08:46
@Rachelint Rachelint marked this pull request as ready for review May 4, 2023 08:56
@Rachelint Rachelint changed the title feat: place table datas into manifest feat: place table datas into manifest, update them together May 4, 2023
@Rachelint Rachelint force-pushed the place-table-datas-into-manifest branch 2 times, most recently from 3c4b191 to 4ebe4d8 Compare May 4, 2023 09:05
@Rachelint Rachelint force-pushed the place-table-datas-into-manifest branch from 4ebe4d8 to b056310 Compare May 4, 2023 09:08
analytic_engine/src/manifest/details.rs Outdated Show resolved Hide resolved
analytic_engine/src/manifest/details.rs Show resolved Hide resolved
analytic_engine/src/manifest/details.rs Outdated Show resolved Hide resolved
analytic_engine/src/instance/open.rs Outdated Show resolved Hide resolved
analytic_engine/src/manifest/meta_edit.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
@Rachelint Rachelint force-pushed the place-table-datas-into-manifest branch from 2be904a to f80557c Compare May 9, 2023 07:54
@Rachelint Rachelint force-pushed the place-table-datas-into-manifest branch from f80557c to 60e6e6c Compare May 9, 2023 07:54
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

@Rachelint Rachelint merged commit be74e29 into apache:main May 9, 2023
chunshao90 pushed a commit to chunshao90/ceresdb that referenced this pull request May 15, 2023
## Which issue does this PR close?

Closes #
Part of apache#799 

## Rationale for this change
Now, we update `TableData` and store its wal seperately. The order of
two operations above is maintained by developer, that will lead to a big
bug if developer is not so careful when modifying related codes.
<!---
Why are you proposing this change? If this is already explained clearly
in the issue, then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand
your changes and offer better suggestions for fixes.
-->

## What changes are included in this PR?
+ Place table datas into manifest.
+ Update it both in memory and storage in the same call.
<!---
There is no need to duplicate the description in the issue here, but it
is sometimes worth providing a summary of the individual changes in this
PR to help reviewers understand the structure.
-->

## Are there any user-facing changes?
None.
<!---
Please mention if:

- there are user-facing changes that need to update the documentation or
configuration.
- this is a breaking change to public APIs
-->

## How does this change test
Test by ut.
<!-- 
Please describe how you test this change (like by unit test case,
integration test or some other ways) if this change has touched the
code.
-->
@Rachelint Rachelint deleted the place-table-datas-into-manifest branch May 27, 2023 12:18
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.

2 participants