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

Add test method for empty pages #260

Merged
merged 3 commits into from
Jan 15, 2024
Merged

Add test method for empty pages #260

merged 3 commits into from
Jan 15, 2024

Conversation

Bouncheck
Copy link
Collaborator

@Bouncheck Bouncheck commented Nov 20, 2023 •
edited by avelanarius
Loading

Adds a test method to SimpleStatementCcmIT that checks if SimpleStatement with page size 1 correctly fetches all rows.

Fixes #254

Bouncheck and others added 2 commits December 5, 2023 02:06
@Bouncheck
Add test method for empty pages
10dd58f 
Adds a test method to SimpleStatementCcmIT that checks
if SimpleStatement with page size 1 correctly fetches all rows.
Covers issue scylladb#254.
@avelanarius @Bouncheck
Properly handle empty pages in MultiPageResultSet
c9f3eb6 
If there are multiple empty pages consecutively, the old
logic did not handle the situation correctly. Use a while
loop to move past many possible empty pages.

(cherry picked from commit 40da47a)
@Bouncheck Bouncheck force-pushed the scylla-4.x-empty-pages-test branch from 0974ec3 to c9f3eb6 Compare December 6, 2023 11:31
@roydahan roydahan requested a review from avelanarius January 4, 2024 13:21
@roydahan
Copy link
Collaborator

roydahan commented Jan 4, 2024

1 check is still failing.

@roydahan
Copy link
Collaborator

roydahan commented Jan 9, 2024

[INFO] Results:
[INFO]
Error: Failures:
Error: DefaultLoadBalancingPolicyIT.should_apply_node_filter:252 expected:<[4]> but was:<[3]>
Error: DefaultLoadBalancingPolicyIT.should_use_round_robin_on_local_dc_when_not_enough_routing_information:140 expected:<...e(endPoint=/127.0.0.[3:9042, hostId=42d20587-0733-4a6b-8971-bbeb6ea22d53, hashCode=7c4139df])> but was:<...e(endPoint=/127.0.0.[2:9042, hostId=e32313f3-c9e7-4a78-a71c-02a40d48657f, hashCode=3d7c33e4])>
Error: Errors:
Error: AddedNodeIT.should_signal_and_create_pool_when_node_gets_added:70 » ConditionTimeout
[INFO]
Error: Tests run: 125, Failures: 2, Errors: 1, Skipped: 37

@avelanarius
Copy link

Debugging the failing test, it doesn't seem to reproduce localy.

@avelanarius
Copy link

The ConditionTimeout error I can reproduce when using master CCM, doesn't reproduce for me with older CCM.

@avelanarius
Copy link

The test (that was failing with ConditionTimeout) passes if I change the timeout to 120 second (from 60 seconds).

Maybe the newer version of CCM doesn't correctly configure the cluster which results in a longer startup time.

@avelanarius
Copy link

Bisecting CCM yields this commit: scylladb/scylla-ccm@dd9ff60

ccmlib: stop passing auto_bootstrap
we stopped handling auto_bootstrap since
scylladb/scylladb@3b1ff90 and the behavior is like it's true, so stop
passing this setting down.

@avelanarius
Copy link

Trying if the tests pass on Github CI with older CCM.

@avelanarius avelanarius force-pushed the scylla-4.x-empty-pages-test branch from f016d45 to 19db6c7 Compare January 12, 2024 15:28
@avelanarius
Copy link

The test pass with older CCM, scylladb/scylla-ccm#550 will fix the master CCM.

@roydahan
Copy link
Collaborator

CCM PR was merged.
All checks passed.

@avelanarius avelanarius force-pushed the scylla-4.x-empty-pages-test branch 2 times, most recently from 828797f to 64263a4 Compare January 15, 2024 16:03
@avelanarius
Pin scylla-ccm version
a06c36b 
Use a pinned version of scylla-ccm instead of master to prevent
accidental breakage of Java Driver CI.
@avelanarius avelanarius force-pushed the scylla-4.x-empty-pages-test branch from 64263a4 to a06c36b Compare January 15, 2024 16:13
@avelanarius avelanarius merged commit 0e84681 into scylladb:scylla-4.x Jan 15, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@avelanarius avelanarius Awaiting requested review from avelanarius

Assignees

@Bouncheck Bouncheck

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Bouncheck @roydahan @avelanarius