mcvsubbu commented on a change in pull request #3796: [PINOT-6] Fix Windows
compatibility of a batch of pinot-core tests
URL: https://github.com/apache/incubator-pinot/pull/3796#discussion_r255597509
##########
File path:
pinot-core/src/main/java/org/apache/pinot/core/segment/index/converter/SegmentV1V2ToV3FormatConverter.java
##########
@@ -85,7 +85,10 @@ public void convert(File v2SegmentDirectory)
File newLocation =
SegmentDirectoryPaths.segmentDirectoryFor(v2SegmentDirectory,
SegmentVersion.v3);
LOGGER.info("v3 segment location for segment: {} is {}",
v2Metadata.getName(), newLocation);
- v3TempDirectory.renameTo(newLocation);
+
+ if (!v3TempDirectory.renameTo(newLocation)) {
+ LOGGER.error("cannot rename {} to {}", v3TempDirectory.getName(),
newLocation.getName());
Review comment:
Not sure under what conditions v3Temp.renameTo will fail, given that
v3TempDir is under the same location.
What exactly is the complaint in windows here?
Also, assuming that this failure is somehow possible, should we not cleanup
by removing v3TempDirectory and throwing an exception? Otherwise we end up
deleting v2 files as well, effectively losing the segment
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@pinot.apache.org
For additional commands, e-mail: dev-h...@pinot.apache.org
