-
Notifications
You must be signed in to change notification settings - Fork 24.9k
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
Reindex API: Disambiguation of requests_per_second #26185
Conversation
Proposal for disambiguation of requests_per_second as discusses in [Reindex API: Reindex task es_rejected_execution_exception search queue failure elastic#26153](elastic#26153 (comment)).
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
I'm not sure about being so detailed in the internal details about how throttling is implemented. But @nik9000 should probably look at this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this a lot better then what I had. I left a note about changing the calculation to make it more clear that the batch write time counts against the wait time. Other things also count against the wait time but they are mostly very fast.
docs/reference/docs/reindex.asciidoc
Outdated
if the `requests_per_second` is set to `500`: | ||
|
||
`wait_time_in_seconds` = `1000` / `500` = `2` seconds | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about something like:
`target_total_time` = `1000` / `500 per second` = `2 seconds`
`wait_time` = `target_total_time` - `batch_write_time` = `2 seconds` - `.5 seconds` = `1.5 seconds`
Updated as per @nik9000 suggestions.
In #26185 we made the description of `requests_per_second` sane for reindex. This improves on the description by using some more common vocabulary ("batch size", etc) and improving the formatting of the example calculation so it stands out and doesn't require scrolling.
Reindex's docs were somewhere between unclear and inaccurate around `requests_per_second`. This makes them much more clear and accurate.
In #26185 we made the description of `requests_per_second` sane for reindex. This improves on the description by using some more common vocabulary ("batch size", etc) and improving the formatting of the example calculation so it stands out and doesn't require scrolling.
Reindex's docs were somewhere between unclear and inaccurate around `requests_per_second`. This makes them much more clear and accurate.
In #26185 we made the description of `requests_per_second` sane for reindex. This improves on the description by using some more common vocabulary ("batch size", etc) and improving the formatting of the example calculation so it stands out and doesn't require scrolling.
Reindex's docs were somewhere between unclear and inaccurate around `requests_per_second`. This makes them much more clear and accurate.
In #26185 we made the description of `requests_per_second` sane for reindex. This improves on the description by using some more common vocabulary ("batch size", etc) and improving the formatting of the example calculation so it stands out and doesn't require scrolling.
Reindex's docs were somewhere between unclear and inaccurate around `requests_per_second`. This makes them much more clear and accurate.
In #26185 we made the description of `requests_per_second` sane for reindex. This improves on the description by using some more common vocabulary ("batch size", etc) and improving the formatting of the example calculation so it stands out and doesn't require scrolling.
Thanks for rewriting the Thanks for doing this. I really didn't have a good way to talk about |
* master: (458 commits) Prevent cluster internal `ClusterState.Custom` impls to leak to a client (elastic#26232) Add packaging test for systemd runtime directive [TEST] Reenable RareClusterStateIt#testDeleteCreateInOneBulk Serialize and expose timeout of acknowledged requests in REST layer (elastic#26189) (refactor) some opportunities to use diamond operator (elastic#25585) [DOCS] Clarified readme for testing a single page Settings: Add keystore.seed auto generated secure setting (elastic#26149) Update version information (elastic#25226) "result" : created -> "result" : "created" (elastic#25446) Set RuntimeDirectory (elastic#23526) Drop upgrade from full cluster restart tests (elastic#26224) Further improve docs for requests_per_second Docs disambiguate reindex's requests_per_second (elastic#26185) [DOCS] Cleanup link for ec2 discovery (elastic#26222) Fix document field equals and hash code test Use holder pattern for lazy deprecation loggers Settings: Add keystore creation to add commands (elastic#26126) Docs: Cleanup docs for ec2 discovery (elastic#26065) Fix NPE when `values` is omitted on percentile_ranks agg (elastic#26046) Several internal improvements to internal test cluster infra (elastic#26214) ...
Proposal for disambiguation of
requests_per_second
as discussed in Reindex API: Reindex task es_rejected_execution_exception search queue failure #26153.@nik9000 As per our discussion in the aforementioned elasticsearch issue, I am looking to disambiguate the
requests_per_second
wait function. I compared some of my results and have a proposal for updating the instructions to reflect my experience and your brief explanation. Here are the results of this unsuccessful scroll batch size of10000
,requests_per_second
of10000
Reindex task:I interpret this formula as
size
/requests_per_second
=wait_time
in seconds. This indeed kind of matches the time in the original post.So in this case I perform the
wait_time
calculation and append it to theTime Working per Scroll
:That fits nicely. So now I will try how I interpret the documentation:
I interpret this formula as either:
batch_time_read_write
- (requests_per_second
*requests_in_the_batch
)requests_per_second
*requests_in_the_batch
) -batch_time_read_write
Neither of these really match, even converting the seconds to nano, it doesn't produce anything meaningful. Of course, this will be different if you are indeed timing the bulk write (
as you may of suggestedYou definitely said this was the case).10000 Failed Task Output
So lets consider the results of a successful scroll batch size of
10000
andrequests_per_second
of5000
Reindex task:size
/requests_per_second
=wait_time
in seconds between eachbulk
:Again, the formula suits the metrics perfectly. You'll also notice the Working EPS is very similar as you'd expect with the same scroll size of
10000
. However, when applying my interpretation of the formula in the documentation:batch_time_read_write
- (requests_per_second
*requests_in_the_batch
)requests_per_second
*requests_in_the_batch
) -batch_time_read_write
Same kind of result again, not what I am experiencing.
5000 Successful Task Output