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

Decouple iceberg commit from risingwave commit. #13899

Closed
Tracked by #10641
liurenjie1024 opened this issue Dec 11, 2023 · 5 comments
Closed
Tracked by #10641

Decouple iceberg commit from risingwave commit. #13899

liurenjie1024 opened this issue Dec 11, 2023 · 5 comments
Assignees
Milestone

Comments

@liurenjie1024
Copy link
Contributor

liurenjie1024 commented Dec 11, 2023

See risingwavelabs/rfcs#78

@github-actions github-actions bot added this to the release-1.6 milestone Dec 11, 2023
@liurenjie1024 liurenjie1024 modified the milestones: release-1.6, release-1.7 Jan 9, 2024
@ZENOTME ZENOTME self-assigned this Mar 4, 2024
@ZENOTME
Copy link
Contributor

ZENOTME commented Mar 6, 2024

I find that after decoupling, it means that the data is not necessary to flush when it meets a checkpoint. To keep the original semantics of flush, we should need to distinguish it from the regular checkpoints. For regular checkpoint, the sink may not flush the data. But for the checkpoint caused by flush, the sink should guarantee to flush the data.

@liurenjie1024
Copy link
Contributor Author

cc @wenym1

@ZENOTME
Copy link
Contributor

ZENOTME commented Mar 6, 2024

Following is the modification probably involved:

  1. BarrierKind <-- add FlushCheckpoint kind
  2. LogWriter::flush_current_epoch <-- add trigger_by_flush param
  3. item for LogStoreIml <-- add trigger_by_flush field
    we only need to modify kv_log_store because in_memory log store can't be used in decouple mode.
  4. LogStoreReadItem <-- add trigger_by_flush

@liurenjie1024
Copy link
Contributor Author

Following is the modification probably involved:

  1. BarrierKind <-- add FlushCheckpoint kind
  2. LogWriter::flush_current_epoch <-- add trigger_by_flush param
  3. item for LogStoreIml <-- add trigger_by_flush field
    we only need to modify kv_log_store because in_memory log store can't be used in decouple mode.
  4. LogStoreReadItem <-- add trigger_by_flush

LGTM

@ZENOTME
Copy link
Contributor

ZENOTME commented May 14, 2024

closed by #15634

@ZENOTME ZENOTME closed this as completed May 14, 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

No branches or pull requests

2 participants