mcvsubbu commented on a change in pull request #3877: Pinot controller side
change to enhance LLC segment metadata upload.
URL: https://github.com/apache/incubator-pinot/pull/3877#discussion_r265169532
##########
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/LLCSegmentCompletionHandlers.java
##########
@@ -360,4 +539,50 @@ private String uploadSegment(FormDataMultiPart multiPart,
String instanceId, Str
multiPart.cleanup();
}
}
+
+ private void localSegmentFileToPinotFSFinalLocation(FileUploadPathProvider
provider, File localTmpFile,
+ String segmentName, String instanceId)
+ throws Exception {
+ final String rawTableName = new LLCSegmentName(segmentName).getTableName();
+ URI segmentFileURI = ControllerConf
+ .getUriFromPath(StringUtil.join("/",
provider.getBaseDataDirURI().toString(), rawTableName, segmentName));
+ PinotFS pinotFS =
PinotFSFactory.create(provider.getBaseDataDirURI().getScheme());
+ // Multiple threads can reach this point at the same time, if the
following scenario happens
+ // The server that was asked to commit did so very slowly (due to network
speeds). Meanwhile the FSM in
+ // SegmentCompletionManager timed out, and allowed another server to
commit, which did so very quickly (somehow
+ // the network speeds changed). The second server made it through the FSM
and reached this point.
+ // The synchronization below takes care that exactly one file gets moved
in place.
+ // There are still corner conditions that are not handled correctly. For
example,
+ // 1. What if the offset of the faster server was different?
+ // 2. We know that only the faster server will get to complete the COMMIT
call successfully. But it is possible
+ // that the race to this statement is won by the slower server, and so
the real segment that is in there is that
+ // of the slower server.
+ // In order to overcome controller restarts after the segment is renamed,
but before it is committed, we DO need to
+ // check for existing segment file and remove it. So, the block cannot be
removed altogether.
+ // For now, we live with these corner cases. Once we have split-commit
enabled and working, this code will no longer
+ // be used.
+ synchronized (SegmentCompletionManager.getInstance()) {
Review comment:
Better yet, let us just get rid of this method and move the whole code to
the caller. That way, the comment and synchronized block reside where they can
be most associated with
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
