Skip to content

test(per table hints): Added a test for per table hints #10965

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Lakshmipathi
Copy link
Contributor

@Lakshmipathi Lakshmipathi commented May 27, 2025

Test per table hints expected_data_size_in_gb option with GrowShrinkNemesis

Testing

PR pre-checks (self review)

  • [x ] I added the relevant backport labels
  • [ x] 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)

@Lakshmipathi Lakshmipathi marked this pull request as ready for review May 27, 2025 14:40
@Lakshmipathi
Copy link
Contributor Author

Running tests once its completed it update job url

Copy link

identified changes in generated code

diff found by running:
bash ./docker/env/hydra.sh update-conf-docs:

 docs/configuration_options.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

@pehala
Copy link
Contributor

pehala commented May 27, 2025

How is the test evaluated? Manually from grafana?

@Lakshmipathi Lakshmipathi force-pushed the wip/per-table-hints-test branch from b2b828b to 352063b Compare May 27, 2025 14:51
@Lakshmipathi
Copy link
Contributor Author

How is the test evaluated? Manually from grafana?

Yes, manually from grafana keyspace charts. If this job runs against non per-tablet hints branch, job will fail with syntax error.

@pehala
Copy link
Contributor

pehala commented May 27, 2025

Yes, manually from grafana keyspace charts. If this job runs against non per-tablet hints branch, job will fail with syntax error.

Please mention it in the docstring for both config and pipeline so it is clear for anyone

EDIT: Also please add correct backport labels

@Lakshmipathi Lakshmipathi force-pushed the wip/per-table-hints-test branch 3 times, most recently from 489fff3 to e2ae99d Compare May 28, 2025 06:43
pehala
pehala previously approved these changes May 28, 2025
@Lakshmipathi Lakshmipathi added backport/none Backport is not required and removed backport/none Backport is not required labels May 28, 2025
@Lakshmipathi Lakshmipathi force-pushed the wip/per-table-hints-test branch from e2ae99d to b1f0ceb Compare May 28, 2025 14:19
Test per table hints expected_data_size_in_gb option with GrowShrinkNemesis

Signed-off-by: Lakshmipathi.Ganapathi <[email protected]>
@Lakshmipathi Lakshmipathi force-pushed the wip/per-table-hints-test branch from b1f0ceb to 9508a1b Compare May 28, 2025 14:23
@pehala
Copy link
Contributor

pehala commented Jun 4, 2025

@scylladb/qa-maintainers please review and merge

Copy link
Contributor

@soyacz soyacz left a comment

Choose a reason for hiding this comment

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

shouldn't this pipeline/test config go under features dir?

# to test hint behavior during cluster scaling operations.
#
# Test Evaluation:
# - Results must be evaluated manually from Grafana keyspace charts
Copy link
Contributor

Choose a reason for hiding this comment

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

need instructions what to evaluate
if instructions are provided - can it be automated?

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree on the instructions, but given what we are trying to do and how uncertain the feature is (they are hints, so not 100% accurate) I think it is not worth the effort to automate it.

Copy link
Contributor

Choose a reason for hiding this comment

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

We have prometheus with metrics and if we expect some specific value ranges we can verify it.
Or if not verify then, at least, report to Argus to the "results" section.

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.

6 participants