jtao15 commented on code in PR #10136:
URL: https://github.com/apache/pinot/pull/10136#discussion_r1080680161
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/SegmentCommitterFactory.java:
##########
@@ -46,25 +46,34 @@ public SegmentCommitterFactory(Logger segmentLogger,
ServerSegmentCompletionProt
_serverMetrics = serverMetrics;
}
+ /**
+ *
+ * @param isSplitCommit Indicates if the controller has enabled split commit
+ * @param params Parameters to use in the Segment completion request
+ * @param controllerVipUrl Unused,
+ * @return
+ * @throws URISyntaxException
+ */
public SegmentCommitter createSegmentCommitter(boolean isSplitCommit,
SegmentCompletionProtocol.Request.Params params,
String controllerVipUrl)
throws URISyntaxException {
if (!isSplitCommit) {
return new DefaultSegmentCommitter(_logger, _protocolHandler, params);
}
- SegmentUploader segmentUploader;
- // TODO Instead of using a peer segment download scheme to control how the
servers do split commit, we should use
- // other configs such as server or controller configs or controller
responses to the servers.
- if (_tableConfig.getValidationConfig().getPeerSegmentDownloadScheme() !=
null) {
- segmentUploader = new
PinotFSSegmentUploader(_indexLoadingConfig.getSegmentStoreURI(),
- PinotFSSegmentUploader.DEFAULT_SEGMENT_UPLOAD_TIMEOUT_MILLIS);
- return new PeerSchemeSplitSegmentCommitter(_logger, _protocolHandler,
params, segmentUploader);
- }
- segmentUploader = new Server2ControllerSegmentUploader(_logger,
_protocolHandler.getFileUploadDownloadClient(),
+ SegmentUploader segmentUploader = new
Server2ControllerSegmentUploader(_logger,
+ _protocolHandler.getFileUploadDownloadClient(),
_protocolHandler.getSegmentCommitUploadURL(params, controllerVipUrl),
params.getSegmentName(),
ServerSegmentCompletionProtocolHandler.getSegmentUploadRequestTimeoutMs(),
_serverMetrics,
_protocolHandler.getAuthProvider());
- return new SplitSegmentCommitter(_logger, _protocolHandler, params,
segmentUploader);
+ boolean uploadToFs =
_tableConfig.getValidationConfig().isUploadToFileSystem();
+ String peerSegmentDownloadScheme =
_tableConfig.getValidationConfig().getPeerSegmentDownloadScheme();
+
+ // TODO: exists for backwards compatibility. remove peerDownloadScheme
non-null check once users have migrated
+ if (uploadToFs || peerSegmentDownloadScheme != null) {
Review Comment:
(nit) Move this condition before `L64` so `segmentUploader` won't be created
twice?
--
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
