Jackie-Jiang commented on code in PR #9604:
URL: https://github.com/apache/pinot/pull/9604#discussion_r997511897
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/indexsegment/immutable/ImmutableSegmentImpl.java:
##########
@@ -283,31 +276,22 @@ public ThreadSafeMutableRoaringBitmap getValidDocIds() {
@Override
public GenericRow getRecord(int docId, GenericRow reuse) {
- try {
- if (_pinotSegmentRecordReader == null) {
- _pinotSegmentRecordReader = new PinotSegmentRecordReader();
- _pinotSegmentRecordReader.init(this);
- }
- _pinotSegmentRecordReader.getRecord(reuse, docId);
+ try (PinotSegmentRecordReader recordReader = new
PinotSegmentRecordReader()) {
Review Comment:
We can directly read records from segment reader, so no need to add another
method into the segment. The problem for partial upsert is that we cannot keep
the segment reader open, or it will consume too much memory. As mentioned in
the description, since partial-upsert update is not on query path, we can merge
it for now, and if people reporting noticeable regression, we can consider not
allowing compression when partial-upsert is enabled.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
