mcvsubbu commented on a change in pull request #5639:
URL: https://github.com/apache/incubator-pinot/pull/5639#discussion_r448536743
##########
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/SegmentCompletionManager.java
##########
@@ -1068,16 +1069,28 @@ private int numReplicasToLookFor() {
_state = State.COMMITTING;
// In case of splitCommit, the segment is uploaded to a unique file name
indicated by segmentLocation,
// so we need to move the segment file to its permanent location first
before committing the metadata.
- if (isSplitCommit) {
+ // The committingSegmentDescriptor is then updated with the permanent
segment location to be saved in metadata
+ // store.
+ // The exception is when the server uses the Peer segment download
scheme, in such case, there is no need to
+ // move the segment.
+ if (isSplitCommit &&
!isPeerSegmentDownloadScheme(committingSegmentDescriptor)) {
Review comment:
we dont need to check the peer download scheme here, since we check and
put the right value while committing metadata in
PinotLLCRealtimeSegmentManager. We can always set the segment location. The if
condition can be as before
##########
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManager.java
##########
@@ -355,8 +355,9 @@ void setIdealState(String realtimeTableName, IdealState
idealState) {
* This method moves the segment file from another location to its permanent
location.
* When splitCommit is enabled, segment file is uploaded to the
segmentLocation in the committingSegmentDescriptor,
* and we need to move the segment file to its permanent location before
committing the segment metadata.
+ * Return the permanent location of the segment if the commit succeeds.
*/
- public void commitSegmentFile(String realtimeTableName,
CommittingSegmentDescriptor committingSegmentDescriptor)
+ public String commitSegmentFile(String realtimeTableName,
CommittingSegmentDescriptor committingSegmentDescriptor)
Review comment:
check the descriptor for peer scheme in this method, and move only if
it is not peer scheme. leave the descriptor.location as is if it is peer
scheme. If not, update the location with the final location in this method. You
can still return void.
----------------------------------------------------------------
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:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
