fullstorydev / solr-bench

Solr benchmarking and load testing harness
Apache License 2.0
17 stars 10 forks source link

Refactoring on index benchmark execution and ControlledExecutor #50

Closed patsonluk closed 1 year ago

patsonluk commented 1 year ago

Description

A refactoring effort to address:

  1. duration-sec is defined for IndexBenchmark but it currently has no effect
  2. Indexing test uses its own implementation for rate control, and does not share the ControlledExecutor which controls duration/rate for querying test
  3. ControlledExecutor has certain discrepancy for the rpm configured vs the actual rpm achieved especially in high throughput scenarios

Solution

  1. Introduced IndexBatchSupplier which "supply" tasks (Callable) which each callable is a index batch writing operation, refactored existing indexing logic to use such supplier and pass that to ControlledExecutor. Such IndexBatchSupplier ensures better separation of concerns of the 2 major tasks:
    1. the docs reading/parsing part from the input doc json file (as DocReader/FileDocReader that respect maxDocs)
    2. the indexing task generation (look up shard, forming batch etc)
  2. Simplified and rewrote some logic in ControlledExecutor such that the controls (duration/rpm) are more well defined and precisely respected.
  3. Simplified RateLimiter to regulate actual rpm closer to the target rpm provided

This is a pretty big refactoring, but I'm hoping this could simplify/solidify the query/index design and code implementation. Could really use some code review and thoughts! @chatman Many thanks! ๐Ÿ™‡๐Ÿผ

patsonluk commented 1 year ago

I haven't tested these changes, but nothing jumps out as obviously wrong. Thanks for tackling this.

Thank you so much @chatman! This is one sizeable refactoring so take your time!

I don't have time yet to write any unit test cases ๐Ÿ˜“ but I did some simple testing on my end (mostly single threaded tho):

  1. Test on having different combination of the restriction factors - duration, maxDocs/runCount. Verified the expected behavior
  2. Verified the rpm rate after the changes.
  3. Ensure index tasks still fulling index all docs when no restrictions are given

I have also started using that for our use case - having light indexing and heavy querying concurrently with fixed duration for both.

Would be great if you can test on your use cases as well! ๐Ÿ’ช๐Ÿผ

chatman commented 1 year ago

Sure, I'll update you by tomorrow on how the testing goes. THanks!

patsonluk commented 1 year ago

@chatman thanks for approving it! Im wondering if you have a chance to run through the test cases on ur end too? ๐Ÿ˜Š

chatman commented 1 year ago

Im wondering if you have a chance to run through the test cases on ur end too?

I'll test them today. Thank your for the patience :-)

chatman commented 1 year ago

@patsonluk , I ran them on a few suites yesterday and didn't notice any problems.

chatman commented 1 year ago

I don't have time yet to write any unit test cases

I've created an issue here, we can revisit later. https://github.com/fullstorydev/solr-bench/issues/53