Jackie-Jiang commented on a change in pull request #7301:
URL: https://github.com/apache/pinot/pull/7301#discussion_r692550023
##########
File path:
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/store/SingleFileIndexDirectory.java
##########
@@ -387,4 +398,66 @@ public boolean isIndexRemovalSupported() {
public String toString() {
return _segmentDirectory.toString() + "/" + _indexFile.toString();
}
+
+ /**
+ * Copy indices, as specified in the Map, from src file to dest file. The
Map contains info
+ * about where to find the index data in the src file, like startOffsets and
data sizes. The
+ * indices are packed together in the dest file, and their positions are
returned.
+ *
+ * @param srcFile contains indices to copy to dest file, and it may contain
other data to leave behind.
+ * @param destFile holds the indices copied from src file, and those indices
appended one after another.
+ * @param indicesToCopy specifies where to find the indices in the src file,
with offset and size info.
+ * @return the offsets and sizes for the indices in the dest file.
+ * @throws IOException from FileChannels upon failure to r/w the index
files, and simply raised to caller.
+ */
+ @VisibleForTesting
+ static List<IndexEntry> copyIndices(File srcFile, File destFile,
Map<IndexKey, IndexEntry> indicesToCopy)
+ throws IOException {
+ // Copy index from original index file and append to temp file.
+ // Keep track of the index entry pointing to the temp index file.
+ List<IndexEntry> retained = new ArrayList<>();
+ long nextOffset = 0;
+ // With FileChannel, we can seek to the data flexibly.
+ try (FileChannel srcCh = new RandomAccessFile(srcFile, "r").getChannel();
+ FileChannel dstCh = new RandomAccessFile(destFile, "rw").getChannel())
{
+ for (Map.Entry<IndexKey, IndexEntry> next : indicesToCopy.entrySet()) {
+ IndexEntry idxPos = next.getValue();
+ copyIndex(srcCh, idxPos._startOffset, idxPos._size, dstCh);
+ retained.add(new IndexEntry(next.getKey(), nextOffset, idxPos._size));
+ nextOffset += idxPos._size;
+ }
Review comment:
I was referring to inlining the method instead of the local variable,
also `indexEntry` already have `indexKey` inside
```suggestion
for (IndexEntry indexEntry : indicesToCopy.values()) {
IndexEntry indexEntry = mapEntry.getValue();
srcCh.transferTo(indexEntry._startOffset, indexEntry._size, dstCh);
retained.add(new IndexEntry(indexEntry._key, nextOffset,
indexEntry._size));
nextOffset += indexEntry._size;
}
```
--
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
