klsince commented on a change in pull request #7301: URL: https://github.com/apache/pinot/pull/7301#discussion_r690825527
########## File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/store/SingleFileIndexDirectory.java ########## @@ -329,47 +323,67 @@ private void persistIndexMap(IndexEntry entry) throws IOException { File mapFile = new File(_segmentDirectory, V1Constants.INDEX_MAP_FILE_NAME); try (PrintWriter writer = new PrintWriter(new BufferedWriter(new FileWriter(mapFile, true)))) { - String startKey = getKey(entry.key.name, entry.key.type.getIndexName(), true); - - StringBuilder sb = new StringBuilder(); - sb.append(startKey).append(" = ").append(entry.startOffset); - writer.println(sb.toString()); - - String endKey = getKey(entry.key.name, entry.key.type.getIndexName(), false); - sb = new StringBuilder(); - sb.append(endKey).append(" = ").append(entry.size); - writer.println(sb.toString()); + persistIndexMap(entry, writer); } } - private String getKey(String column, String indexName, boolean isStartOffset) { - return column + MAP_KEY_SEPARATOR + indexName + MAP_KEY_SEPARATOR + (isStartOffset ? "startOffset" : "size"); - } - private String allocationContext(IndexKey key) { return this.getClass().getSimpleName() + key.toString(); } + /** + * This method sweeps the indices marked for removal. Exception is simply bubbled up w/o + * trying to recover disk states from failure. This method is expected to run during segment + * reloading, which has failure handling by creating a backup folder before doing reloading. + */ + private void cleanupRemovedIndices() + throws IOException { + if (!_shouldCleanupRemovedIndices) { + return; + } + + File tmpIdxFile = new File(_segmentDirectory, V1Constants.INDEX_FILE_NAME + ".tmp"); + // Sort indices by column name and index type while copying, so that the + // new index_map file is easy to inspect for troubleshooting. + List<IndexEntry> retained = copyIndicesTo(_indexFile, new TreeMap<>(_columnEntries), tmpIdxFile); Review comment: I kinda prefer to leave this to caller, so that they can control data layout; and leave this method to do one thing - copying. lemme know your 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