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/03/05 00:52:07 UTC
[GitHub] [incubator-pinot] fx19880617 opened a new pull request #5114:
Bugfixing the issue for ThreadLocal DocIdSet in ExpressionFilterOperator
fx19880617 opened a new pull request #5114: Bugfixing the issue for ThreadLocal DocIdSet in ExpressionFilterOperator
URL: https://github.com/apache/incubator-pinot/pull/5114
Bug fixing for: https://github.com/apache/incubator-pinot/issues/5103
Root cause is that we use `THREAD_LOCAL_DOC_IDS` in `DocIdSetOperator`.
However expression evaluation through `ExpressionFilterOperator` will construct `DocIdSetOperator` multiple times with FilterOperator to reset the `THREAD_LOCAL_DOC_IDS` values.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] fx19880617 commented on issue #5114: Bugfixing
the issue for ThreadLocal DocIdSet in ExpressionFilterOperator
Posted by GitBox <gi...@apache.org>.
fx19880617 commented on issue #5114: Bugfixing the issue for ThreadLocal DocIdSet in ExpressionFilterOperator
URL: https://github.com/apache/incubator-pinot/pull/5114#issuecomment-595411323
> IMO, the clean fix should be making another DocIdSetOperator which directly works on bitmap. We can fix the bug first and optimize later, your choice.
I suggest we fix this issue first then optimize it later on
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #5114: Bugfixing the
issue for ThreadLocal DocIdSet in ExpressionFilterOperator
Posted by GitBox <gi...@apache.org>.
jackjlli merged pull request #5114: Bugfixing the issue for ThreadLocal DocIdSet in ExpressionFilterOperator
URL: https://github.com/apache/incubator-pinot/pull/5114
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull
request #5114: Bugfixing the issue for ThreadLocal DocIdSet in
ExpressionFilterOperator
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #5114: Bugfixing the issue for ThreadLocal DocIdSet in ExpressionFilterOperator
URL: https://github.com/apache/incubator-pinot/pull/5114#discussion_r388058386
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/filter/ExpressionFilterOperator.java
##########
@@ -196,7 +196,7 @@ public BlockDocIdIterator iterator() {
int _endDocId;
//used only in next() and advance methods
int _currentBlockStartDocId = -1;
- int _currentBlockEndDocId = 0;
+ int _currentBlockEndDocId = -1;
Review comment:
I would recommend making both endDocId and this exclusive. Currently the numDocsScanned value is incorrect.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull
request #5114: Bugfixing the issue for ThreadLocal DocIdSet in
ExpressionFilterOperator
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #5114: Bugfixing the issue for ThreadLocal DocIdSet in ExpressionFilterOperator
URL: https://github.com/apache/incubator-pinot/pull/5114#discussion_r388058619
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/DocIdSetOperator.java
##########
@@ -69,7 +75,7 @@ protected DocIdSetBlock getNextBlock() {
}
int pos = 0;
- int[] docIds = THREAD_LOCAL_DOC_IDS.get();
+ int[] docIds = _threadLocal? THREAD_LOCAL_DOC_IDS.get(): new int[DocIdSetPlanNode.MAX_DOC_PER_CALL];
Review comment:
```suggestion
int[] docIds = _threadLocal? THREAD_LOCAL_DOC_IDS.get(): new int[_maxSizeOfDocIdSet];
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org