You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "vvivekiyer (via GitHub)" <gi...@apache.org> on 2023/07/19 05:28:55 UTC
[GitHub] [pinot] vvivekiyer opened a new pull request, #11131: Fix bug in reload segment when one or more indexes are removed.
vvivekiyer opened a new pull request, #11131:
URL: https://github.com/apache/pinot/pull/11131
- This PR fixes the bug reported in https://github.com/apache/pinot/issues/11130
- The PR also fixes other existing calls to `FileChannel.transferTo`
--
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] vvivekiyer commented on a diff in pull request #11131: Fix bug in reload segment when one or more indexes are removed.
Posted by "vvivekiyer (via GitHub)" <gi...@apache.org>.
vvivekiyer commented on code in PR #11131:
URL: https://github.com/apache/pinot/pull/11131#discussion_r1268451707
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/inv/geospatial/BaseH3IndexCreator.java:
##########
@@ -157,9 +157,12 @@ void generateIndexFile()
FileChannel bitmapOffsetFileChannel = new RandomAccessFile(_bitmapOffsetFile, "r").getChannel();
FileChannel bitmapValueFileChannel = new RandomAccessFile(_bitmapValueFile, "r").getChannel()) {
indexFileChannel.write(headerBuffer);
- dictionaryFileChannel.transferTo(0, _dictionaryFile.length(), indexFileChannel);
- bitmapOffsetFileChannel.transferTo(0, _bitmapOffsetFile.length(), indexFileChannel);
- bitmapValueFileChannel.transferTo(0, _bitmapValueFile.length(), indexFileChannel);
+ org.apache.pinot.common.utils.FileUtils.transferBytes(dictionaryFileChannel, 0, _dictionaryFile.length(),
Review Comment:
We have another FileUtils in `org.apache.commons.io.FileUtils`. Hence used the full path here.
Having different class names could be a separate effort.
--
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 #11131: Fix bug in reload segment when one or more indexes are removed.
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #11131:
URL: https://github.com/apache/pinot/pull/11131#issuecomment-1641476191
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/11131?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#11131](https://app.codecov.io/gh/apache/pinot/pull/11131?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (0081164) into [master](https://app.codecov.io/gh/apache/pinot/commit/9374964ad4b5797329333157fcbb1de56cc66657?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (9374964) will **increase** coverage by `0.00%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## master #11131 +/- ##
=========================================
Coverage 0.11% 0.11%
=========================================
Files 2191 2148 -43
Lines 118007 115692 -2315
Branches 17868 17584 -284
=========================================
Hits 137 137
+ Misses 117850 115535 -2315
Partials 20 20
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1temurin11 | `?` | |
| integration1temurin17 | `?` | |
| integration1temurin20 | `?` | |
| integration2temurin11 | `?` | |
| integration2temurin17 | `?` | |
| unittests1temurin11 | `?` | |
| unittests1temurin17 | `?` | |
| unittests1temurin20 | `?` | |
| unittests2temurin11 | `0.11% <0.00%> (-0.01%)` | :arrow_down: |
| unittests2temurin17 | `0.11% <0.00%> (-0.01%)` | :arrow_down: |
| unittests2temurin20 | `?` | |
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=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://app.codecov.io/gh/apache/pinot/pull/11131?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...reator/impl/inv/geospatial/BaseH3IndexCreator.java](https://app.codecov.io/gh/apache/pinot/pull/11131?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2NyZWF0b3IvaW1wbC9pbnYvZ2Vvc3BhdGlhbC9CYXNlSDNJbmRleENyZWF0b3IuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...nt/creator/impl/inv/json/BaseJsonIndexCreator.java](https://app.codecov.io/gh/apache/pinot/pull/11131?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2NyZWF0b3IvaW1wbC9pbnYvanNvbi9CYXNlSnNvbkluZGV4Q3JlYXRvci5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...ment/creator/impl/text/NativeTextIndexCreator.java](https://app.codecov.io/gh/apache/pinot/pull/11131?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2NyZWF0b3IvaW1wbC90ZXh0L05hdGl2ZVRleHRJbmRleENyZWF0b3IuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [.../local/segment/store/SingleFileIndexDirectory.java](https://app.codecov.io/gh/apache/pinot/pull/11131?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L3N0b3JlL1NpbmdsZUZpbGVJbmRleERpcmVjdG9yeS5qYXZh) | `0.00% <0.00%> (ø)` | |
... and [131 files with indirect coverage changes](https://app.codecov.io/gh/apache/pinot/pull/11131/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
: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=apache)
--
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] vvivekiyer commented on a diff in pull request #11131: Fix bug in reload segment when one or more indexes are removed.
Posted by "vvivekiyer (via GitHub)" <gi...@apache.org>.
vvivekiyer commented on code in PR #11131:
URL: https://github.com/apache/pinot/pull/11131#discussion_r1268451707
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/inv/geospatial/BaseH3IndexCreator.java:
##########
@@ -157,9 +157,12 @@ void generateIndexFile()
FileChannel bitmapOffsetFileChannel = new RandomAccessFile(_bitmapOffsetFile, "r").getChannel();
FileChannel bitmapValueFileChannel = new RandomAccessFile(_bitmapValueFile, "r").getChannel()) {
indexFileChannel.write(headerBuffer);
- dictionaryFileChannel.transferTo(0, _dictionaryFile.length(), indexFileChannel);
- bitmapOffsetFileChannel.transferTo(0, _bitmapOffsetFile.length(), indexFileChannel);
- bitmapValueFileChannel.transferTo(0, _bitmapValueFile.length(), indexFileChannel);
+ org.apache.pinot.common.utils.FileUtils.transferBytes(dictionaryFileChannel, 0, _dictionaryFile.length(),
Review Comment:
We have another FileUtils in `org.apache.commons.io.FileUtils`. Hence used the full path here.
Refactoring to have different class names could be a separate effort.
--
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] jackjlli merged pull request #11131: Fix bug in reload segment when one or more indexes are removed.
Posted by "jackjlli (via GitHub)" <gi...@apache.org>.
jackjlli merged PR #11131:
URL: https://github.com/apache/pinot/pull/11131
--
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] somandal commented on a diff in pull request #11131: Fix bug in reload segment when one or more indexes are removed.
Posted by "somandal (via GitHub)" <gi...@apache.org>.
somandal commented on code in PR #11131:
URL: https://github.com/apache/pinot/pull/11131#discussion_r1268273117
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/inv/geospatial/BaseH3IndexCreator.java:
##########
@@ -157,9 +157,12 @@ void generateIndexFile()
FileChannel bitmapOffsetFileChannel = new RandomAccessFile(_bitmapOffsetFile, "r").getChannel();
FileChannel bitmapValueFileChannel = new RandomAccessFile(_bitmapValueFile, "r").getChannel()) {
indexFileChannel.write(headerBuffer);
- dictionaryFileChannel.transferTo(0, _dictionaryFile.length(), indexFileChannel);
- bitmapOffsetFileChannel.transferTo(0, _bitmapOffsetFile.length(), indexFileChannel);
- bitmapValueFileChannel.transferTo(0, _bitmapValueFile.length(), indexFileChannel);
+ org.apache.pinot.common.utils.FileUtils.transferBytes(dictionaryFileChannel, 0, _dictionaryFile.length(),
Review Comment:
nit: can we add this as an import?
--
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