Repository navigation
[tests] Wait until users, roles and grants reach all cluster replicas - #3285
Merged
Merged
Conversation
On the 3-replica test cluster, users, roles, settings profiles and grants
are kept in the replicated user directory. Each replica updates its own
copy from Keeper asynchronously, and nginx sends each request to any
replica. A login, GRANT or DROP ... IF EXISTS right after a change can go
to a replica that does not have the change yet: AUTHENTICATION_FAILED,
UNKNOWN_ROLE, or ACCESS_ENTITY_ALREADY_EXISTS.
Add BaseIntegrationTest.waitForAllReplicas(). It checks system.<table>
on every replica through clusterAllReplicas('test_cluster', ...) with a
limited number of attempts, and does nothing outside cluster mode. Call
it after each access change that a later request depends on, and give
each test its own user, role and profile names, dropped in finally.
Part of: #3267
Fixes: #3237
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
6 tasks done
Contributor
Author
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 6b30c52. Configure here.
|
Client V2 CoverageCoverage Report
Class Coverage
|
JDBC V2 CoverageCoverage Report
Class Coverage
|
JDBC V1 CoverageCoverage Report
Class Coverage
|
Client V1 CoverageCoverage Report
Class Coverage
|
joe-clickhouse
approved these changes
Nov 2, 2026
Contributor
Author
|
Closes #3267 |
This was referenced Nov 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Summary
See original PR #3277
Reworking in the way it uses proper tools
Fixes #3237.
Checklist
Delete items not relevant to your PR:
Note
Low Risk
Changes are limited to integration test infrastructure and assertions; production client or JDBC code is untouched.
Overview
Adds
waitForAllReplicastest helpers that pollsystem.*viaclusterAllReplicasuntil every node in the three-node test cluster reports the expected row count, then no-ops in single-node mode.ClickHouseServerForTestimplements the polling loop (up to ~10s);BaseIntegrationTestexposes it, and jdbc-v2 adds an overload that runs the check through client-v2 because the legacy HTTP client is not on that module’s classpath.Integration tests that create users, roles, settings profiles, or grants now call the waiter after DDL instead of assuming immediate visibility (including replacing manual retry loops in credential-change tests). Tests also use unique suffixed names and
finallycleanup so parallel or repeated cluster runs do not collide on fixed identifiers likesome_user.Reviewed by Cursor Bugbot for commit 6b30c52. Bugbot is set up for automated code reviews on this repo. Configure here.