You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "INNOCENT-BOY (via GitHub)" <gi...@apache.org> on 2023/04/15 10:17:07 UTC
[GitHub] [pinot] INNOCENT-BOY opened a new pull request, #10620: bugfix: Add write and read lock on RealtimeLuceneTextIndex to keep t…
INNOCENT-BOY opened a new pull request, #10620:
URL: https://github.com/apache/pinot/pull/10620
bugfix: Add write and read lock on RealtimeLuceneTextIndex to keep t…
Our team create an Text Index for one column. When the amount of concurrency is small, there is no problem with the query. However as the amount of concurrency increases, various problems caused by concurrency will now pop up. Then we will add write/read lock to LuceneTextIndex.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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] [pinot] mayankshriv commented on pull request #10620: bugfix: Add write and read lock on RealtimeLuceneTextIndex to keep t…
Posted by "mayankshriv (via GitHub)" <gi...@apache.org>.
mayankshriv commented on PR #10620:
URL: https://github.com/apache/pinot/pull/10620#issuecomment-1509869802
Any performance numbers to share, given that this can lead to high thread contention for high throughput use cases?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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] [pinot] INNOCENT-BOY commented on pull request #10620: bugfix: Add write and read lock on RealtimeLuceneTextIndex to keep t…
Posted by "INNOCENT-BOY (via GitHub)" <gi...@apache.org>.
INNOCENT-BOY commented on PR #10620:
URL: https://github.com/apache/pinot/pull/10620#issuecomment-1510421762
> The fix looks good. Have you verified this in your environment?
Hi @Jackie-Jiang ,although i tried some ways to add readwritelock to lucenetextindex, it doesn't work. Currently the only way to avoid generating parseException is to add ReentrantLock. But as we all know, that is not a perfect way, especially handling a lots of read requests. I will continue to debug this issue. And If you have any good idea, please let me know.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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] [pinot] Jackie-Jiang merged pull request #10620: bug fix: to keep QueryParser thread safe when handling many read requests on class RealtimeLuceneTextIndex
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang merged PR #10620:
URL: https://github.com/apache/pinot/pull/10620
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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] [pinot] INNOCENT-BOY commented on pull request #10620: bug fix: to keep QueryParser thread safe when handling many read requests on class RealtimeLuceneTextIndex
Posted by "INNOCENT-BOY (via GitHub)" <gi...@apache.org>.
INNOCENT-BOY commented on PR #10620:
URL: https://github.com/apache/pinot/pull/10620#issuecomment-1513504795
> Hi @Jackie-Jiang , I performed lots of tests. The reason why report parseException is not thread content between read threads and write thread. The root cause is QueryParser is not thread safe, so we can't share QueryParser instance between all request on the same consuming segment. And I have given an suitable solution. Each request on consuming segments that have text index will generate their own QueryParser instance. This instance will destroy after executing parse statement. The speed of parse is very fast. So this fix vesion won't bring burden to JVM memory. Meanwhile it won't make read path single-threaded. Hope your review and reply, @Jackie-Jiang .
<img width="708" alt="image" src="https://user-images.githubusercontent.com/38196564/232850363-5ab117dc-7f81-426d-b970-ff54d4032290.png">
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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] [pinot] codecov-commenter commented on pull request #10620: bugfix: Add write and read lock on RealtimeLuceneTextIndex to keep t…
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #10620:
URL: https://github.com/apache/pinot/pull/10620#issuecomment-1509726154
## [Codecov](https://codecov.io/gh/apache/pinot/pull/10620?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#10620](https://codecov.io/gh/apache/pinot/pull/10620?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7f5da60) into [master](https://codecov.io/gh/apache/pinot/commit/c7e05a7b58f5435080ab26e9ef8888e2b07dd974?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c7e05a7) will **decrease** coverage by `13.78%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## master #10620 +/- ##
=============================================
- Coverage 27.66% 13.89% -13.78%
- Complexity 58 439 +381
=============================================
Files 2090 2052 -38
Lines 112578 110530 -2048
Branches 16972 16730 -242
=============================================
- Hits 31145 15354 -15791
- Misses 78333 93919 +15586
+ Partials 3100 1257 -1843
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests2 | `13.89% <0.00%> (?)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/10620?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...me/impl/invertedindex/RealtimeLuceneTextIndex.java](https://codecov.io/gh/apache/pinot/pull/10620?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9yZWFsdGltZS9pbXBsL2ludmVydGVkaW5kZXgvUmVhbHRpbWVMdWNlbmVUZXh0SW5kZXguamF2YQ==) | `0.00% <0.00%> (ø)` | |
... and [976 files with indirect coverage changes](https://codecov.io/gh/apache/pinot/pull/10620/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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] [pinot] INNOCENT-BOY commented on pull request #10620: bugfix: Add write and read lock on RealtimeLuceneTextIndex to keep t…
Posted by "INNOCENT-BOY (via GitHub)" <gi...@apache.org>.
INNOCENT-BOY commented on PR #10620:
URL: https://github.com/apache/pinot/pull/10620#issuecomment-1509884791
@mayankshriv Hi we haven't have any test numbers now，but we used the same lock with other index，such as inverted index that submitted by @Jackie-Jiang
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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] [pinot] INNOCENT-BOY commented on pull request #10620: bugfix: Add write and read lock on RealtimeLuceneTextIndex to keep t…
Posted by "INNOCENT-BOY (via GitHub)" <gi...@apache.org>.
INNOCENT-BOY commented on PR #10620:
URL: https://github.com/apache/pinot/pull/10620#issuecomment-1513019946
Hi @Jackie-Jiang , I perfomed lots of tests. The reason why report parseException is not thread content between read threads and write thread. The root cause is QueryParser is not thread safe, so we can't share QueryParser instance between all request on the same consuming segment. And I have given an suitable solution. Each request on consuming segments that have text index will generate their own QueryParser instance. This instance will destroy after parse. This parse is very fast. So this fix vesion can't bring burden to JVM memory. Meanwhile it won't make read path single-threaded. Hope your review and reply, @Jackie-Jiang .
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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