liran-funaro opened a new pull request #10593:
URL: https://github.com/apache/druid/pull/10593
<!-- Thanks for trying to help us make Apache Druid be the best it can be!
Please fill out as much of the following information as is possible (where
relevant, and remove it when irrelevant) to help make the intention and scope
of this PR clear in order to ease review. -->
Fixes #10494.
### Description
<!-- Describe the goal of this PR, what problem are you fixing. If there is
a corresponding issue (referenced above), it's not necessary to repeat the
description here, however, you may choose to keep one summary sentence. -->
**Note:** This PR only affects tests and benchmarks.
#10335 added a per incremental-index builder, but the parent class builder
(IncrementalIndex.Builder) was not removed to avoid 100+ lines changes in the
test code.
This PR removes IncrementalIndex.Builder and refactor all its usage (only in
tests the test/benchmarks code) to be parametrized so it will test/benchmark
both builder implementations (on-heap and off-heap).
<!-- Describe your patch: what did you change in code? How did you fix the
problem? -->
<!-- If there are several relatively logically separate changes in this PR,
create a mini-section for each of them. For example: -->
#### Add test cases for each index type
All tests that are relevant to the incremental index were modified. The
modifications include the parametrization of the tests for all
incremental-index implementations: on-heap and off-heap. In addition, this PR
includes a bug fix in `OffheapIncrementalIndex` that was found using these
tests.
To support this, a new helper class was added: `IncrementalIndexCreator`.
This class handle creating the appropriate index according to its name, and
closing it at the end of each test.
#### Add benchmark cases for each index type
All the benchmarks that are relevant to the incremental index was added an
incremental-index parametrization: on-heap or off-heap.
In addition, some of these benchmarks were modified to resolve some issues
that were encountered.
We list here the additional modifications we made to some of the benchmarks.
* Add some additional parametrization:
- rollup opportunity for the row generator
- number of rows per segment
- query order: descending/ascending
* Modify the parametrization of the rollup opportunity for the row
generator
- From implicit names (`{"none", "moderate", "high"}`) to explicit
number of repeated timestamps (`{"0", "1000", "10000"}`)
* Add a missing `tearDown()` procedure
* Properly close the queryable index in the `tearDown()` procedure
* Moved any temporary folder creation and deletion to the
`setup()`/`tearDown()` methods so they would not affect the measurements of the
results
* Use a predefined seed for reproducible results, to be compliant with
most benchmarks
* Add scopes (`@State(Scope.Benchmark)`) that allow us to test the
incremental index without the overhead of the setup procedure of the queryable
index benchmark
- One scope for benchmarking queries on the incremental index
- One scope for benchmarking queries on the queryable index
In addition, to reduce code duplications, a few methods were added to
`DataGenerator`:
1. `void addToIndex(IncrementalIndex<?> index, int numOfRows)`: adds rows
from this generator to an existing index
1. `List<InputRow> toList(int numOfRows)`: adds rows from this generator to
a new list
<!--
In each section, please describe design decisions made, including:
- Choice of algorithms
- Behavioral aspects. What configuration values are acceptable? How are
corner cases and error conditions handled, such as when there are insufficient
resources?
- Class organization and design (how the logic is split between classes,
inheritance, composition, design patterns)
- Method organization and design (how the logic is split between methods,
parameters and return types)
- Naming (class, method, API, configuration, HTTP endpoint, names of
emitted metrics)
-->
<!-- It's good to describe an alternative design (or mention an alternative
name) for every design (or naming) decision point and compare the alternatives
with the designs that you've implemented (or the names you've chosen) to
highlight the advantages of the chosen designs and names. -->
<!-- If there was a discussion of the design of the feature implemented in
this PR elsewhere (e. g. a "Proposal" issue, any other issue, or a thread in
the development mailing list), link to that discussion from this PR description
and explain what have changed in your final design compared to your original
proposal or the consensus version in the end of the discussion. If something
hasn't changed since the original discussion, you can omit a detailed
discussion of those aspects of the design here, perhaps apart from brief
mentioning for the sake of readability of this PR description. -->
<!-- Some of the aspects mentioned above may be omitted for simple and small
changes. -->
<hr>
This PR has:
- [X] been self-reviewed.
- [ ] added documentation for new or modified features or behaviors.
- [X] added Javadocs for most classes and all non-trivial methods. Linked
related entities via Javadoc links.
- [ ] added or updated version, license, or notice information in
[licenses.yaml](https://github.com/apache/druid/blob/master/licenses.yaml)
- [X] added comments explaining the "why" and the intent of the code
wherever would not be obvious for an unfamiliar reader.
- [X] added unit tests or modified existing tests to cover new code paths,
ensuring the threshold for [code
coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md)
is met.
- [ ] added integration tests.
- [ ] been tested in a test Druid cluster.
<!-- Check the items by putting "x" in the brackets for the done things. Not
all of these items apply to every PR. Remove the items which are not done or
not relevant to the PR. None of the items from the checklist above are strictly
necessary, but it would be very helpful if you at least self-review the PR. -->
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
