richardstartin commented on a change in pull request #8211:
URL: https://github.com/apache/pinot/pull/8211#discussion_r808500979
##########
File path:
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/indexsegment/mutable/MutableSegmentImpl.java
##########
@@ -465,20 +468,19 @@ public void addExtraColumns(Schema newSchema) {
_logger.info("Newly added columns: " +
_newlyAddedColumnsFieldMap.toString());
}
- // NOTE: Okay for single-writer
- @SuppressWarnings("NonAtomicOperationOnVolatileField")
@Override
public boolean index(GenericRow row, @Nullable RowMetadata rowMetadata)
throws IOException {
boolean canTakeMore;
+ int numDocsIndexed = _numDocsIndexed;
Review comment:
snapshots `_numDocsIndexed` for a few reasons:
* This was actually safe, but code that generates warnings misleads people.
It looks wrong and makes the reader ask questions.
* On ARM (increasingly relevant hardware) reading a volatile variable incurs
an actual LoadStore barrier.
--
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]
