Exclude tasks based on serverless status by pquentin · Pull Request #1760 · elastic/rally

pquentin · 2023-08-10T13:34:39Z

When running Rally benchmarks on Elasticsearch Serverless, some operations do not make sense depending on the operator status, such as force merge. To make running benchmarks simpler, this pull requests skips some operations automatically based on the operator status of the user.

I've tested this with the PMC track that has a challenge with a parallel operation by explicitly setting serverless.operator to False:

esrally race --track-path=$HOME/src/rally-tracks/pmc --test-mode --target-hosts=... --pipeline=benchmark-only --client-options=serverless.json --user-tags="intention:rally-serverless" --on-error=abort --challenge=indexing-querying

It prints the following at the beginning of the race:

[INFO] Race id is [71666d4e-121b-47a8-b7c2-8b2550434e95]
[INFO] Treating parallel task in challenge [indexing-querying] as public.
[INFO] Excluding [put-settings], [check-cluster-health], [force-merge], [wait-until-merges-finish] as challenge [indexing-querying] is run on serverless.
[INFO] Racing on track [pmc], challenge [indexing-querying] and car ['external'] with version [8.10.0].

TODO:

Fix rally-tracks-compat
Add escape hatch

Sub commands to test:

gbanasiak

The overall approach looks great. I did first round of testing and found one issue with custom operations (they are filtered, but shouldn't).

esrally/track/loader.py

esrally/track/track.py

gbanasiak · 2023-08-11T07:15:45Z

esrally/rally.py

            cfg.add(config.Scope.applicationOverride, "system", "list.to_date", args.to_date)
            configure_mechanic_params(args, cfg, command_requires_car=False)
            configure_track_params(arg_parser, args, cfg, command_requires_track=False)
+            configure_default_serverless_params(cfg)


Is there a benefit of calling configure_default_serverless_params 3 times in list, delete and info instead of calling it once at the top of dispatch_sub_command ?

Not really, it felt weird to set those values for commands where it's not needed. Happy to change this.

I'm happy to keep current form as long as all sub-commands have been tested. Our test coverage is not complete hence the thought of moving it to the top.

esrally/track/loader.py

gbanasiak · 2023-08-11T08:29:46Z

Forgot to mention I also went through all operation types and compared with serverless protection status as of July 27th and found no discrepancies.

This is not required as it's the whole tuple that needs to be unique, but it's less confusing that way.

dliappis

@pquentin

I am probably missing some background which is not covered in the PR description, so I may be barking at the wrong tree, but I had issues testing this PR.

I've tested this with the PMC track that has a challenge with a parallel operation by explicitly setting serverless.operator to False:

It's unclear to me how to reproduce what you did here.
Do I have to set the corresponding setting in rally.ini as supported since #1756?
Looking at https://github.com/elastic/rally/pull/1760/files#diff-9cd59b2fffb801ee46e1819d3dc79a407b1b5784a09d535b3ff68820f9c4102dR202 it appears it's not needed to be done explicitly any more.
But in any case with (see below) or without this setting

$ tail -4 ~/.rally/rally.ini 
[driver]
serverless.operator=False
serverless.mode=True

and targeting a serverless ES using the pmc track I see:

[INFO] Race id is [8aebdb5b-d810-4a82-aa50-b96b9590ef3f]                                                                                                                                                                                      
[INFO] Treating parallel task in challenge [indexing-querying] as public.
[INFO] Downloading track data (5.5 GB total size)                                 [100.0%]                                                                                                                                                    
[INFO] Decompressing track data from [/home/esbench/.rally/benchmarks/data/pmc/documents.json.bz2] to [/home/esbench/.rally/benchmarks/data/pmc/documents.json] (resulting size: [21.66] GB) ... [OK]
[INFO] Preparing file offset table for [/home/esbench/.rally/benchmarks/data/pmc/documents.json] ... [OK]
[INFO] Racing on track [pmc], challenge [indexing-querying] and car ['external'] with version [8.10.0].

which differs from your output, namely doesn't show:

[INFO] Excluding [put-settings], [check-cluster-health], [force-merge], [wait-until-merges-finish] as challenge [indexing-querying] is run on serverless.

Do I also need a modification in the track and/or pass the track parameter run-on-serverless that is introduced in this PR?

pquentin · 2023-08-15T08:19:17Z

You do need to manually set serverless.operator to False in the source code. This will be detected in a later pull request.

dliappis · 2023-08-15T08:20:06Z

No but you need to manually set serverless.operator to False, sorry. This will be detected in a later pull request.

Where, in rally.ini under driver? If yes, I did that, but couldn't get the behavior you described i.e. [INFO] Excluding [put-settings], [check-cluster-health], [force-merge], [wait-until-merges-finish] as challenge [indexing-querying] is run on serverless.

dliappis · 2023-08-15T08:27:38Z

You do need to manually set serverless.operator to False in the source code. This will be detected in a later pull request.

Ok after explicitly setting False in https://github.com/elastic/rally/pull/1760/files#diff-9cd59b2fffb801ee46e1819d3dc79a407b1b5784a09d535b3ff68820f9c4102dR204, I managed to get the expected output

[INFO] Treating parallel task in challenge [indexing-querying] as public.
[INFO] Excluding [put-settings], [check-cluster-health], [force-merge], [wait-until-merges-finish] as challenge [indexing-querying] is run on serverless.

It's unclear to me why explicitly setting that value in rally.ini under [driver] wouldn't achieve the same result though.

pquentin · 2023-08-22T13:42:20Z

I fixed reading from rally.ini. Please take another look!

b-deam · 2023-08-23T01:57:14Z

tests/track/loader_test.py

+        }
+        reader = loader.TrackSpecificationReader()
+        full_track = reader("unittest", copy.deepcopy(track_specification), "/mappings")
+        filtered = self.filter(full_track, serverless_mode=True, serverless_operator=True)


Nit: can we use filtered_track consistently between the tests?

b-deam

Nice work! I really like how readable it is, given the complexities here.

I tested it using this invocation:

$ esrally race --pipeline=benchmark-only --client-options='{"default": { "use_ssl": true, "verify_certs": false, "basic_auth_user":"elastic","basic_auth_password":"changeme", "timeout": 60, "headers": { "X-Found-Cluster": "b1ec3121-f6d4-4f62-9e57-422581b3bb65.es"}}}' --target-hosts="https://my.elb.eu-west-1.amazonaws.com" --track-params="number_of_replicas:1,bulk_indexing_clients:4" --track nyc_taxis --challenge append-no-conflicts --kill-running-processes --telemetry=blob-store-stats --test-mode

With serverless.operator=True:

[...]

[INFO] Race id is [8a0017a4-fb1a-4392-9ab2-dc9e505c403d]
[INFO] Racing on track [nyc_taxis], challenge [append-no-conflicts] and car ['external'] with version [8.10.0].

Running delete-index                                                           [100% done]
Running create-index                                                           [100% done]
Running check-cluster-health                                                   [100% done]
Running index                                                                  [100% done]
Running refresh-after-index                                                    [100% done]
Running force-merge                                                            [100% done]
Running refresh-after-force-merge                                              [100% done]
Running wait-until-merges-finish                                               [100% done]
Running default_no_target                                                      [100% done]
Running default                                                                [100% done]
Running default_1k                                                             [100% done]
Running range                                                                  [100% done]
Running distance_amount_agg                                                    [100% done]
Running autohisto_agg                                                          [100% done]
Running date_histogram_agg                                                     [100% done]

[...]

and with serverless.operator=False:

[...]

[INFO] Race id is [66e95a15-78b9-4815-a710-ad6c0d298865]
[INFO] Excluding [check-cluster-health], [force-merge], [wait-until-merges-finish] as challenge [append-no-conflicts] is run on serverless.
[INFO] Racing on track [nyc_taxis], challenge [append-no-conflicts] and car ['external'] with version [8.10.0].

Running delete-index                                                           [100% done]
Running create-index                                                           [100% done]
Running index                                                                  [100% done]
Running refresh-after-index                                                    [100% done]
Running refresh-after-force-merge                                              [100% done]
Running default_no_target                                                      [100% done]
Running default                                                                [100% done]
Running default_1k                                                             [100% done]
Running range                                                                  [100% done]
Running distance_amount_agg                                                    [100% done]
Running autohisto_agg                                                          [100% done]
Running date_histogram_agg                                                     [100% done]

[...]

Exclude tasks based on serverless status

2bd1c13

pquentin added this to the 2.9.0 milestone Aug 10, 2023

pquentin requested review from dliappis, gareth-ellis and gbanasiak August 10, 2023 13:34

pquentin self-assigned this Aug 10, 2023

pquentin added 2 commits August 11, 2023 09:02

Fix esrally list tracks

ec6e0b3

Fix esrally info

f25170b

gbanasiak reviewed Aug 11, 2023

View reviewed changes

pquentin added 7 commits August 11, 2023 13:22

Add notes about raw-request

1dea719

Explain why IntEnum and auto() are important

784b56d

Don't print serverless message if not task was excluded

b374794

Stop filtering user-provided operations

a6c54dc

Add escape hatch for serverless filtering

bf6f932

Fix OperationType ordering

7f86b64

Ensure all OperationType ids are unique

a3ceb81

This is not required as it's the whole tuple that needs to be unique, but it's less confusing that way.

dliappis reviewed Aug 15, 2023

View reviewed changes

pquentin modified the milestones: 2.9.0, 2.10.0 Aug 16, 2023

Read severless configuration from rally.ini again

35038c9

pquentin requested a review from b-deam August 22, 2023 13:41

b-deam reviewed Aug 23, 2023

View reviewed changes

b-deam approved these changes Aug 23, 2023

View reviewed changes

Rename filtered to filtered_track

7929811

pquentin merged commit be63a9a into elastic:master Aug 23, 2023

pquentin deleted the skip-serverless-operations branch August 23, 2023 09:00

pquentin mentioned this pull request Aug 23, 2023

Detect serverless operator status automatically #1768

Merged

pquentin modified the milestones: 2.10.0, 2.9.0 Aug 24, 2023

Conversation

pquentin commented Aug 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gbanasiak left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gbanasiak Aug 11, 2023

Choose a reason for hiding this comment

Uh oh!

pquentin Aug 11, 2023

Choose a reason for hiding this comment

Uh oh!

gbanasiak Aug 11, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gbanasiak commented Aug 11, 2023

Uh oh!

dliappis left a comment

Choose a reason for hiding this comment

Uh oh!

pquentin commented Aug 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dliappis commented Aug 15, 2023

Uh oh!

dliappis commented Aug 15, 2023

Uh oh!

pquentin commented Aug 22, 2023

Uh oh!

b-deam Aug 23, 2023

Choose a reason for hiding this comment

Uh oh!

b-deam left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

pquentin commented Aug 10, 2023 •

edited

Loading

pquentin commented Aug 15, 2023 •

edited

Loading