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

test(customd2): add custom_d2/workload1 test configurations #8951

Merged

Conversation

vponomaryov
Copy link
Contributor

@vponomaryov vponomaryov commented Oct 9, 2024

First configuration is small - 3 DCs, 3 nodes per each.

Second configuration is big - 5DCs, 12 nodes per each.
This big setup never reached the Test start stage
because of the very slow bootstrap process in a multi-dc environment.

Ref: #19131

Testing

PR pre-checks (self review)

  • I added the relevant backport labels
  • I didn't leave commented-out/debugging code

Reminders

  • Add New configuration option and document them (in sdcm/sct_config.py)
  • Add unit tests to cover my changes (under unit-test/ folder)
  • Update the Readme/doc folder relevant to this change (if needed)

@vponomaryov vponomaryov added backport/2024.2 Need backport to 2024.2 backport/2024.1 Need backport to 2024.1 backport/6.1 Need backport to 6.1 backport/6.2 labels Oct 9, 2024
@vponomaryov vponomaryov requested review from fruch and roydahan October 9, 2024 14:39
@vponomaryov vponomaryov force-pushed the add-custom-d1-w2-test-configuration branch from bdf009f to 831b632 Compare October 10, 2024 09:00
- >-
latte run --tag latte --duration 180m --request-timeout 60 --retry-interval '2s,10s'
--sampling 5s --threads 30 --connections 3 --concurrency 180 --rate 15000 -P offset=0
--function custom -P row_count=50100100 -P codes="\"T13F1\"" -P print_applied_func_names=2 --consistency=QUORUM
Copy link
Contributor

Choose a reason for hiding this comment

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

why in some command we specify --consistency=QUORUM and in some we don't ?

I would guess that the default is LOCAL_QUORUM

I guess there is some assumption here that one remember by heart codes for queries...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Have you read comment on line 139?
It directly influences your question.
First command out of 5 writes to it's own region, other 4 loaders of the first region write to per-dc tables of other 4 regions. Such actiosn requires usage of non-local QUORUM for these other 4 loaders.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In places where we do not specify CL we depend on the default value which is local_quorum.

Copy link
Contributor

Choose a reason for hiding this comment

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

I've read the comment, the part about why CL usage need to be different, is not easily deduced from that comment. one need a clear mental picture of the whole setup in mind, and that really hard in this case.

maybe we should consider adding some overall description on qa-internal repo, and point to it.
I'm guessing a diagram for this case might be helpful.

@vponomaryov vponomaryov force-pushed the add-custom-d1-w2-test-configuration branch from 831b632 to 305fc1f Compare October 10, 2024 09:05
@vponomaryov vponomaryov force-pushed the add-custom-d1-w2-test-configuration branch 2 times, most recently from 95090f1 to f130457 Compare October 10, 2024 11:24
@vponomaryov vponomaryov force-pushed the add-custom-d1-w2-test-configuration branch from f130457 to fe4c75b Compare October 29, 2024 20:28
@vponomaryov
Copy link
Contributor Author

60 nodes scenario passed using fix from Asias (scylladb/scylladb#21207):

@vponomaryov vponomaryov force-pushed the add-custom-d1-w2-test-configuration branch 2 times, most recently from ae01500 to d36bcc5 Compare December 10, 2024 13:05
@vponomaryov
Copy link
Contributor Author

The latest state of the 5dcs was tested here:

Using the unmerged PR (via BYO) needed for the enterprise versions: https://github.com/scylladb/scylla-enterprise/pull/5039

@vponomaryov vponomaryov requested a review from fruch December 10, 2024 13:09
@vponomaryov
Copy link
Contributor Author

The current configuration is suitable for current Scylla master versions only.

Following Scylla PR backports are needed for any other Scylla version than current master:

SCT_GCE_IMAGE_DB=image SCT_GCE_DATACENTER="us-east1 us-west1 eu-north1" SCT_SCYLLA_REPO='http://downloads.scylladb.com.s3.amazonaws.com/rpm/centos/scylla-2021.1.repo' ./sct.py lint-yamls -b gce -i '3dcs,24h-multidc,large-cluster,counters-multidc,cdc-8h-multi-dc' -e 'docker,azure,shutdown'
OUT=$(($OUT + $?))

echo "multi dc yamls with 5 regions"
Copy link
Contributor

Choose a reason for hiding this comment

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

oh my...
I really hate this shell script

one day we should move it into python code, to speed things up, and make it a bit less annoying

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree

fruch
fruch previously approved these changes Dec 10, 2024
Copy link
Contributor

@fruch fruch left a comment

Choose a reason for hiding this comment

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

LGTM

@fruch
Copy link
Contributor

fruch commented Dec 10, 2024

@vponomaryov

you have one unittest failure, cause of the new scylla configuration:

unit_tests/test_scylla_yaml.py::ScyllaYamlTest::test_scylla_yaml 

First configuration is "small" - 3 DCs, 3 nodes per each.
Second configuration is "big" - 5DCs, 12 nodes per each.

Ref: #19131
@vponomaryov vponomaryov force-pushed the add-custom-d1-w2-test-configuration branch from a563c31 to e3bdd1a Compare December 17, 2024 17:52
Copy link
Contributor

@fruch fruch left a comment

Choose a reason for hiding this comment

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

LGTM

@vponomaryov
Copy link
Contributor Author

@fruch , @roydahan can we merge this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/6.1-done Commit backported to 6.1 backport/6.2-done backport/2024.1 Need backport to 2024.1 backport/2024.2-done Commit backported to 2024.2 promoted-to-master
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants