You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2020/08/17 05:35:39 UTC

[GitHub] [incubator-pinot] jasperjiaguo opened a new pull request #5875: Config recommender patch

jasperjiaguo opened a new pull request #5875:
URL: https://github.com/apache/incubator-pinot/pull/5875


   Fixed a few typos in the recommender 
   Added default values to avoid recommender accessing null values
   Reduced the time complexity for index recommendation


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #5875: Config recommender patch

Posted by GitBox <gi...@apache.org>.
jackjlli commented on a change in pull request #5875:
URL: https://github.com/apache/incubator-pinot/pull/5875#discussion_r473276920



##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/recommender/rules/utils/QueryInvertedSortedIndexRecommender.java
##########
@@ -585,7 +573,9 @@ else if (type == Predicate.Type.IN || type == Predicate.Type.NOT_IN) {
 
       boolean isFirst = false;
       List<String> values = (type == Predicate.Type.IN)?((InPredicate) predicate).getValues():((NotInPredicate) predicate).getValues();
-      if (values.get(RecommenderConstants.FIRST).equals(RecommenderConstants.IN_PREDICATE_ESTIMATE_LEN_FLAG) ||
+      if(values.size()==1){

Review comment:
       should put a space between `)` and `{`

##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/recommender/rules/io/params/RecommenderConstants.java
##########
@@ -32,7 +32,7 @@
     public static final double DEFAULT_THRESHOLD_MIN_AND_PREDICATE_INCREMENTAL_VOTE = 0.6d;
     public static final double DEFAULT_THRESHOLD_RATIO_MIN_AND_PREDICATE_TOP_CANDIDATES = 0.8d;
     public static final double DEFAULT_THRESHOLD_RATIO_MIN_GAIN_DIFF_BETWEEN_ITERATION = 0.05d;
-    public static final int DEFAULT_MAX_NUM_ITERATION_WITHOUT_GAIN = 3;
+    public static final int DEFAULT_MAX_NUM_ITERATION_WITHOUT_GAIN = 2;

Review comment:
       Can we add the link of this class to the doc so that we know this is the default values?

##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/recommender/rules/impl/InvertedSortedIndexJointRule.java
##########
@@ -197,28 +194,48 @@ public PredicateParseResult findOptimalCombination(List<List<PredicateParseResul
    * {@link PredicateParseResult#getnESI()} the nESI before applying any index
    * {@link PredicateParseResult#getnESIWithIdx()} the estimated nESI after applying optimal indices
    */
-  public PredicateParseResult evaluateCombination(int n, int r, List<List<PredicateParseResult>> parsedQuery) {
-    List<int[]> combinationIntArrays = generateCombinations(n, r);
-    LOGGER.debug("combinationIntArrays {}", combinationIntArrays);
+  public PredicateParseResult evaluateCombination(final int n, final int r, List<List<PredicateParseResult>> parsedQuery) {
+    FixedLenBitset usedCols = new FixedLenBitset(n);
+    parsedQuery.forEach(list -> list.stream()
+        .filter(predicateParseResult -> (predicateParseResult.getCandidateDims().getCardinality() <= r))
+        .forEach(predicateParseResult -> usedCols.union(predicateParseResult.getCandidateDims())));
+    LOGGER.debug("totalUsed {}", usedCols.getCardinality());
+
+    List<Integer> usedColIDs = usedCols.getOffsets();
+    int nCapped = usedColIDs.size();
+    int rCapped = Math.min(r, nCapped);
+
+    int[] idToColID = new int[nCapped];
+    for (int i = 0; i < nCapped; i++) {
+      idToColID[i] = usedColIDs.get(i);
+    }
+
 
     // Enumerate all possible combinations of r-sized set, which will be applied indices on
-    List<FixedLenBitset> combinations = combinationIntArrays.parallelStream().map(combinationIntArray -> {
-      FixedLenBitset indices = new FixedLenBitset(n);
-      for (int j = 0; j < r; j++) {
-        indices.add(combinationIntArray[j]);
-      }
-      return indices;
-    }).collect(Collectors.toList());
+    List<int[]> combinationIntArrays = generateCombinations(nCapped, rCapped);
+
+//     Enumerate all possible combinations of r-sized set, which will be applied indices on
+//    List<FixedLenBitset> combinations = combinationIntArrays.parallelStream().map(combinationIntArray -> {

Review comment:
       Maybe we should remove this comments?




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [incubator-pinot] codecov-commenter commented on pull request #5875: Config recommender patch

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #5875:
URL: https://github.com/apache/incubator-pinot/pull/5875#issuecomment-676757756


   # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5875?src=pr&el=h1) Report
   > Merging [#5875](https://codecov.io/gh/apache/incubator-pinot/pull/5875?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/1beaab59b73f26c4e35f3b9bc856b03806cddf5a&el=desc) will **decrease** coverage by `22.76%`.
   > The diff coverage is `52.69%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-pinot/pull/5875/graphs/tree.svg?width=650&height=150&src=pr&token=4ibza2ugkz)](https://codecov.io/gh/apache/incubator-pinot/pull/5875?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff             @@
   ##           master    #5875       +/-   ##
   ===========================================
   - Coverage   66.44%   43.68%   -22.77%     
   ===========================================
     Files        1075     1182      +107     
     Lines       54773    61672     +6899     
     Branches     8168     9427     +1259     
   ===========================================
   - Hits        36396    26941     -9455     
   - Misses      15700    32308    +16608     
   + Partials     2677     2423      -254     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | #integration | `43.68% <52.69%> (?)` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/5875?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...ot/broker/broker/AllowAllAccessControlFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/5875/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL0FsbG93QWxsQWNjZXNzQ29udHJvbEZhY3RvcnkuamF2YQ==) | `100.00% <ø> (ø)` | |
   | [.../helix/BrokerUserDefinedMessageHandlerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/5875/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0Jyb2tlclVzZXJEZWZpbmVkTWVzc2FnZUhhbmRsZXJGYWN0b3J5LmphdmE=) | `52.83% <0.00%> (-13.84%)` | :arrow_down: |
   | [...ava/org/apache/pinot/client/AbstractResultSet.java](https://codecov.io/gh/apache/incubator-pinot/pull/5875/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Fic3RyYWN0UmVzdWx0U2V0LmphdmE=) | `26.66% <0.00%> (-30.48%)` | :arrow_down: |
   | [.../main/java/org/apache/pinot/client/Connection.java](https://codecov.io/gh/apache/incubator-pinot/pull/5875/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Nvbm5lY3Rpb24uamF2YQ==) | `22.22% <0.00%> (-26.62%)` | :arrow_down: |
   | [.../org/apache/pinot/client/ResultTableResultSet.java](https://codecov.io/gh/apache/incubator-pinot/pull/5875/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L1Jlc3VsdFRhYmxlUmVzdWx0U2V0LmphdmE=) | `24.00% <0.00%> (-10.29%)` | :arrow_down: |
   | [.../org/apache/pinot/common/lineage/LineageEntry.java](https://codecov.io/gh/apache/incubator-pinot/pull/5875/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbGluZWFnZS9MaW5lYWdlRW50cnkuamF2YQ==) | `0.00% <0.00%> (ø)` | |
   | [...apache/pinot/common/lineage/LineageEntryState.java](https://codecov.io/gh/apache/incubator-pinot/pull/5875/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbGluZWFnZS9MaW5lYWdlRW50cnlTdGF0ZS5qYXZh) | `0.00% <0.00%> (ø)` | |
   | [...rg/apache/pinot/common/lineage/SegmentLineage.java](https://codecov.io/gh/apache/incubator-pinot/pull/5875/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbGluZWFnZS9TZWdtZW50TGluZWFnZS5qYXZh) | `0.00% <0.00%> (ø)` | |
   | [...ache/pinot/common/lineage/SegmentLineageUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/5875/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbGluZWFnZS9TZWdtZW50TGluZWFnZVV0aWxzLmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [...ot/common/messages/RoutingTableRebuildMessage.java](https://codecov.io/gh/apache/incubator-pinot/pull/5875/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWVzc2FnZXMvUm91dGluZ1RhYmxlUmVidWlsZE1lc3NhZ2UuamF2YQ==) | `0.00% <0.00%> (ø)` | |
   | ... and [1097 more](https://codecov.io/gh/apache/incubator-pinot/pull/5875/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5875?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5875?src=pr&el=footer). Last update [2b58bfb...70960d9](https://codecov.io/gh/apache/incubator-pinot/pull/5875?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [incubator-pinot] jackjlli merged pull request #5875: Config recommender patch

Posted by GitBox <gi...@apache.org>.
jackjlli merged pull request #5875:
URL: https://github.com/apache/incubator-pinot/pull/5875


   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [incubator-pinot] jasperjiaguo commented on a change in pull request #5875: Config recommender patch

Posted by GitBox <gi...@apache.org>.
jasperjiaguo commented on a change in pull request #5875:
URL: https://github.com/apache/incubator-pinot/pull/5875#discussion_r473318904



##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/recommender/rules/io/params/RecommenderConstants.java
##########
@@ -32,7 +32,7 @@
     public static final double DEFAULT_THRESHOLD_MIN_AND_PREDICATE_INCREMENTAL_VOTE = 0.6d;
     public static final double DEFAULT_THRESHOLD_RATIO_MIN_AND_PREDICATE_TOP_CANDIDATES = 0.8d;
     public static final double DEFAULT_THRESHOLD_RATIO_MIN_GAIN_DIFF_BETWEEN_ITERATION = 0.05d;
-    public static final int DEFAULT_MAX_NUM_ITERATION_WITHOUT_GAIN = 3;
+    public static final int DEFAULT_MAX_NUM_ITERATION_WITHOUT_GAIN = 2;

Review comment:
       Sure

##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/recommender/rules/impl/InvertedSortedIndexJointRule.java
##########
@@ -197,28 +194,48 @@ public PredicateParseResult findOptimalCombination(List<List<PredicateParseResul
    * {@link PredicateParseResult#getnESI()} the nESI before applying any index
    * {@link PredicateParseResult#getnESIWithIdx()} the estimated nESI after applying optimal indices
    */
-  public PredicateParseResult evaluateCombination(int n, int r, List<List<PredicateParseResult>> parsedQuery) {
-    List<int[]> combinationIntArrays = generateCombinations(n, r);
-    LOGGER.debug("combinationIntArrays {}", combinationIntArrays);
+  public PredicateParseResult evaluateCombination(final int n, final int r, List<List<PredicateParseResult>> parsedQuery) {
+    FixedLenBitset usedCols = new FixedLenBitset(n);
+    parsedQuery.forEach(list -> list.stream()
+        .filter(predicateParseResult -> (predicateParseResult.getCandidateDims().getCardinality() <= r))
+        .forEach(predicateParseResult -> usedCols.union(predicateParseResult.getCandidateDims())));
+    LOGGER.debug("totalUsed {}", usedCols.getCardinality());
+
+    List<Integer> usedColIDs = usedCols.getOffsets();
+    int nCapped = usedColIDs.size();
+    int rCapped = Math.min(r, nCapped);
+
+    int[] idToColID = new int[nCapped];
+    for (int i = 0; i < nCapped; i++) {
+      idToColID[i] = usedColIDs.get(i);
+    }
+
 
     // Enumerate all possible combinations of r-sized set, which will be applied indices on
-    List<FixedLenBitset> combinations = combinationIntArrays.parallelStream().map(combinationIntArray -> {
-      FixedLenBitset indices = new FixedLenBitset(n);
-      for (int j = 0; j < r; j++) {
-        indices.add(combinationIntArray[j]);
-      }
-      return indices;
-    }).collect(Collectors.toList());
+    List<int[]> combinationIntArrays = generateCombinations(nCapped, rCapped);
+
+//     Enumerate all possible combinations of r-sized set, which will be applied indices on
+//    List<FixedLenBitset> combinations = combinationIntArrays.parallelStream().map(combinationIntArray -> {

Review comment:
       Done

##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/recommender/rules/utils/QueryInvertedSortedIndexRecommender.java
##########
@@ -585,7 +573,9 @@ else if (type == Predicate.Type.IN || type == Predicate.Type.NOT_IN) {
 
       boolean isFirst = false;
       List<String> values = (type == Predicate.Type.IN)?((InPredicate) predicate).getValues():((NotInPredicate) predicate).getValues();
-      if (values.get(RecommenderConstants.FIRST).equals(RecommenderConstants.IN_PREDICATE_ESTIMATE_LEN_FLAG) ||
+      if(values.size()==1){

Review comment:
       Reformatted




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org