Creating a pluggable interface for Table config tuner

[icefury71]

Description

Related to step 1 of #6254

This diff adds an annotation type to register table config tuner methods with the Pinot controller. When a new table is being added, this tuner can be used to customize the table config.

Upgrade Notes

Does this PR prevent a zero down-time upgrade?
No

Does this PR fix a zero-downtime upgrade introduced earlier?
No

Does this PR otherwise need attention when creating release notes?
Yes (will add release notes in final commit)

[mcvsubbu]

thanks for creating the issue. I added a comment there

[codecov-io]

Codecov Report

Merging #6255 (cd8dc06) into master (1beaab5) will decrease coverage by 1.05%.
The diff coverage is 45.12%.

@@            Coverage Diff             @@
##           master    #6255      +/-   ##
==========================================
- Coverage   66.44%   65.39%   -1.06%     
==========================================
  Files        1075     1291     +216     
  Lines       54773    62143    +7370     
  Branches     8168     9013     +845     
==========================================
+ Hits        36396    40640    +4244     
- Misses      15700    18613    +2913     
- Partials     2677     2890     +213     

Flag	Coverage Δ
unittests	65.39% <45.12%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...e/pinot/broker/api/resources/PinotBrokerDebug.java	0.00% <0.00%> (-79.32%)	⬇️
...ot/broker/broker/AllowAllAccessControlFactory.java	71.42% <ø> (-28.58%)	⬇️
.../helix/BrokerUserDefinedMessageHandlerFactory.java	33.96% <0.00%> (-32.71%)	⬇️
...ker/routing/instanceselector/InstanceSelector.java	100.00% <ø> (ø)
...ava/org/apache/pinot/client/AbstractResultSet.java	66.66% <0.00%> (+9.52%)	⬆️
.../main/java/org/apache/pinot/client/Connection.java	35.55% <0.00%> (-13.29%)	⬇️
...inot/client/JsonAsyncHttpPinotClientTransport.java	10.90% <0.00%> (-51.10%)	⬇️
...not/common/assignment/InstancePartitionsUtils.java	73.80% <ø> (+0.63%)	⬆️
...common/config/tuner/NoOpTableTableConfigTuner.java	100.00% <ø> (ø)
...ot/common/config/tuner/RealTimeAutoIndexTuner.java	100.00% <ø> (ø)
... and 1140 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 75f9fd3...cd8dc06. Read the comment docs.

[mcvsubbu]

How does this play with AutoAddInvertedIndexTool we have?

[siddharthteotia]

We have a full fledged rule engine to automatically recommend table configs (indexing and other things to begin with). We should enhance that instead of implementing a new one. Starting point for the rule engine driver
https://github.com/apache/incubator-pinot/blob/master/pinot-controller/src/main/java/org/apache/pinot/controller/recommender/RecommenderDriver.java

[siddharthteotia]

https://github.com/apache/incubator-pinot/blob/master/pinot-tools/src/main/java/org/apache/pinot/tools/AutoAddInvertedIndexTool.java
and
https://github.com/apache/incubator-pinot/blob/master/pinot-controller/src/main/java/org/apache/pinot/controller/util/AutoAddInvertedIndex.java

How does this play with AutoAddInvertedIndexTool we have?

@mcvsubbu , The correct code to begin with for the newly implemented rule engine is RecommenderDriver.

[siddharthteotia]

@icefury71 , can we see how the RecommendationEngine (the rule engine that was recently implemented by @jasperjiaguo ) be plugged into this interface or at least we can identify if the abstractions are correct. IIUC, this PR provides a pluggable interface that can use a rule engine and the reommendations generated by the engine to modify the table config?

The existing rule engine takes a couple of inputs (at bare minimum the following)

• Schema
• Query pattern

It generates a series of recommendations on how to tune the table config.

It will be great to see if there is an impedance mismatch between the abstractions sooner and converge. cc @jasperjiaguo @mcvsubbu

[mcvsubbu]

As pointed out by @siddharthteotia we already have a rule engine in place.
We also have a realtime prov helper that accepts a sample segment.

Can we follow the interface/factory pattern here, so that we can ingest whatever factory we want? That way, if a rule engine needs more information (sample segments, sample queries, etc) to make the decision, it can.

Also, we should be able to chain these (and each member of the chain should modify only those that are not already touched).

[icefury71]

@siddharthteotia @mcvsubbu since we're using an annotation type, there is no such fixed interface. The parameter and return type validations are all done via code which can be evolved over time. I'm happy to do that in this PR - but wanted to be clear.

@mcvsubbu the annotation type was a recommendation from @kishoreg instead of the old interface / factory pattern. This approach definitely makes it easy to register tuners and is much more flexible. Let me know if this is an issue.

[mcvsubbu]

@siddharthteotia @mcvsubbu since we're using an annotation type, there is no such fixed interface. The parameter and return type validations are all done via code which can be evolved over time. I'm happy to do that in this PR - but wanted to be clear.

@mcvsubbu the annotation type was a recommendation from @kishoreg instead of the old interface / factory pattern. This approach definitely makes it easy to register tuners and is much more flexible. Let me know if this is an issue.

I am sure I am missing something.

How would I register a tuner that takes a URI for a sample segment, and an object with a query pattern in it (say, has queries, and percentages)

[siddharthteotia]

@icefury71 , Approved this. We can make the existing Tuner (RecommenderDriver) extend the Tuner interface, implement init(), apply() and register. I highly suggest to explore this tuner and see if this can be further improved/optimized. It is essentially a rule engine where you can add your own custom rules that will fire to generate a series of recommendations.
We will add some more functionality to it in the next quarter for realtime.