Skip to content

test: add integration test for logstore scale-in / out #21781

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

Draft
wants to merge 20 commits into
base: main
Choose a base branch
from

Conversation

kwannoel
Copy link
Contributor

@kwannoel kwannoel commented May 8, 2025

I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.

What's changed and what's your intention?

Last part of unaligned join #20408.

Kill some nodes, wait for worker to be expired, which triggers scale-in. Restart the worker nodes after scale-in, to trigger scale-out again.

Finally, check that the data in the logstore is correct.

Checklist

  • I have written necessary rustdoc comments.
  • I have added necessary unit tests and integration tests.
  • I have added test labels as necessary.
  • I have added fuzzing tests or opened an issue to track them.
  • My PR contains breaking changes.
  • My PR changes performance-critical code, so I will run (micro) benchmarks and present the results.
  • I have checked the Release Timeline and Currently Supported Versions to determine which release branches I need to cherry-pick this PR into.

Documentation

  • My PR needs documentation updates.
Release note

@github-actions github-actions bot added the component/test Component: Testing framework & misc tests not belonging to any specific component. label May 8, 2025
@kwannoel kwannoel requested a review from wenym1 May 8, 2025 08:43

[meta]
meta_leader_lease_secs = 10
max_heartbeat_interval_secs = 5
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copied from ci-sim, but with this additional configuration, to force worker node to be evicted.

Copy link
Contributor

Choose a reason for hiding this comment

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

If this heartbeat value was too large previously, should we set this value directly in the origin ci-sim and reuse the same config file so that more corner case can be tested in the integration tests and recovery tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, we can change it in a subsequent PR to avoid scope creep

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

license-eye has checked 5711 files.

Valid Invalid Ignored Fixed
2397 1 3313 0
Click to see the invalid file list
  • src/tests/simulation/tests/integration_tests/log_store/utils.rs
Use this command to fix any missing license headers
```bash

docker run -it --rm -v $(pwd):/github/workspace apache/skywalking-eyes header fix

</details>

@@ -27,27 +27,7 @@ use risingwave_simulation::cluster::{Cluster, ConfigPath, Configuration, KillOpt
use risingwave_simulation::ctl_ext::predicate::identity_contains;
use tokio::time::sleep;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Refactored utilies from this file to utils.rs

@kwannoel kwannoel requested a review from shanicky May 8, 2025 08:44
@kwannoel kwannoel mentioned this pull request May 8, 2025
8 tasks
@kwannoel kwannoel changed the title test: test log store scale-in / out test: add integration test for logstore scale-in / out May 9, 2025

[meta]
meta_leader_lease_secs = 10
max_heartbeat_interval_secs = 5
Copy link
Contributor

Choose a reason for hiding this comment

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

If this heartbeat value was too large previously, should we set this value directly in the origin ci-sim and reuse the same config file so that more corner case can be tested in the integration tests and recovery tests?

@kwannoel kwannoel force-pushed the kwannoel/synced-log-store-scale-in branch 4 times, most recently from 9d778c0 to 47485a5 Compare May 19, 2025 13:55
Copy link
Contributor Author

kwannoel commented May 19, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@kwannoel
Copy link
Contributor Author

Mark as draft, so we can just test the scale-in test.

@kwannoel kwannoel force-pushed the kwannoel/synced-log-store-scale-in branch from e2505a8 to d7ec856 Compare May 21, 2025 07:16
@kwannoel kwannoel changed the base branch from main to graphite-base/21781 May 21, 2025 15:28
@kwannoel kwannoel force-pushed the graphite-base/21781 branch from 024e422 to 76056fd Compare May 21, 2025 15:28
@kwannoel kwannoel force-pushed the kwannoel/synced-log-store-scale-in branch from f79c380 to fce6930 Compare May 21, 2025 15:28
@kwannoel kwannoel changed the base branch from graphite-base/21781 to yiming/fix-committed-table-watermark-update May 21, 2025 15:28
@kwannoel kwannoel force-pushed the kwannoel/synced-log-store-scale-in branch from fce6930 to c078236 Compare May 21, 2025 15:30
@kwannoel kwannoel changed the base branch from yiming/fix-committed-table-watermark-update to main May 21, 2025 15:32
@kwannoel kwannoel changed the base branch from main to yiming/fix-committed-table-watermark-update May 21, 2025 15:33
@kwannoel kwannoel changed the base branch from yiming/fix-committed-table-watermark-update to graphite-base/21781 May 21, 2025 15:41
@kwannoel kwannoel force-pushed the graphite-base/21781 branch from 76056fd to 7863c52 Compare May 21, 2025 15:41
@kwannoel kwannoel force-pushed the kwannoel/synced-log-store-scale-in branch from c078236 to fce6930 Compare May 21, 2025 15:41
@kwannoel kwannoel changed the base branch from graphite-base/21781 to main May 21, 2025 15:41
@kwannoel kwannoel force-pushed the kwannoel/synced-log-store-scale-in branch 2 times, most recently from 25cc76d to 1c870ed Compare May 21, 2025 15:44
@kwannoel kwannoel force-pushed the kwannoel/synced-log-store-scale-in branch from 1c870ed to 69c7538 Compare May 22, 2025 07:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants