vvivekiyer commented on code in PR #9510:
URL: https://github.com/apache/pinot/pull/9510#discussion_r995567992


##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/ForwardIndexHandler.java:
##########
@@ -173,7 +168,143 @@ private void rewriteRawForwardIndex(String column, 
SegmentDirectory.Writer segme
       IndexCreatorProvider indexCreatorProvider)
       throws Exception {
     Preconditions.checkState(_segmentMetadata.getVersion() == 
SegmentVersion.v3);
+    ColumnMetadata existingColMetadata = 
_segmentMetadata.getColumnMetadataFor(column);
+    boolean isSingleValue = existingColMetadata.isSingleValue();
+
+    if (isSingleValue) {
+      rewriteRawSVForwardIndex(column, segmentWriter, indexCreatorProvider);
+    } else {
+      rewriteRawMVForwardIndex(column, segmentWriter, indexCreatorProvider);
+    }
+  }
+
+  private void rewriteRawMVForwardIndex(String column, SegmentDirectory.Writer 
segmentWriter,
+      IndexCreatorProvider indexCreatorProvider)
+      throws Exception {
+    ColumnMetadata existingColMetadata = 
_segmentMetadata.getColumnMetadataFor(column);
+    File indexDir = _segmentMetadata.getIndexDir();
+    String segmentName = _segmentMetadata.getName();
+    File inProgress = new File(indexDir, column + ".fwd.inprogress");
+    File fwdIndexFile = new File(indexDir, column + 
V1Constants.Indexes.RAW_MV_FORWARD_INDEX_FILE_EXTENSION);
+
+    if (!inProgress.exists()) {
+      // Marker file does not exist, which means last run ended normally.
+      // Create a marker file.
+      FileUtils.touch(inProgress);
+    } else {
+      // Marker file exists, which means last run was interrupted.
+      // Remove forward index if exists.
+      FileUtils.deleteQuietly(fwdIndexFile);
+    }
+
+    LOGGER.info("Creating new forward index for segment={} and column={}", 
segmentName, column);
+     Map<String, ChunkCompressionType> compressionConfigs = 
_indexLoadingConfig.getCompressionConfigs();
+     Preconditions.checkState(compressionConfigs.containsKey(column));
+    // At this point, compressionConfigs is guaranteed to contain the column.
+    ChunkCompressionType newCompressionType = compressionConfigs.get(column);
+
+    int numDocs = existingColMetadata.getTotalDocs();
+
+    try (ForwardIndexReader reader = 
LoaderUtils.getForwardIndexReader(segmentWriter, existingColMetadata)) {
+      int lengthOfLongestEntry = reader.getLengthOfLongestEntry();
+      Preconditions.checkState(lengthOfLongestEntry >= 0,
+          "lengthOfLongestEntry cannot be negative. segment=" + segmentName + 
" column={}" + column);
+      int maxNumberOfMVEntries = 
existingColMetadata.getMaxNumberOfMultiValues();
+      int maxRowLengthInBytes = lengthOfLongestEntry - (Integer.BYTES * 
maxNumberOfMVEntries) - Integer.BYTES;

Review Comment:
   Good point. I've added comments explaining the logic. I've also moved the 
math inside static functions in `MultiValueVarByteRawIndexCreator`. 
   
   IMO, it's still a little confusing because of our existing naming - e.g 
`lengthOfLongestEntry` `maxRowLengthInBytes`, etc.  Cleaning these names 
everywhere is a larger undertaking and better left as a future cleanup. 
Thoughts?



-- 
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-unsubscr...@pinot.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org

Reply via email to