[GitHub] [incubator-pinot] chenboat commented on a change in pull request #4914: [POC] By-passing deep-store requirement for Realtime segment completion
chenboat commented on a change in pull request #4914: [POC] By-passing
deep-store requirement for Realtime segment completion
URL: https://github.com/apache/incubator-pinot/pull/4914#discussion_r398960371
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeTableDataManager.java
##
@@ -257,21 +280,78 @@ public void downloadAndReplaceSegment(String
segmentName, LLCRealtimeSegmentZKMe
final File segmentFolder = new File(_indexDir, segmentName);
FileUtils.deleteQuietly(segmentFolder);
try {
-
SegmentFetcherFactory.getInstance().getSegmentFetcherBasedOnURI(uri).fetchSegmentToLocal(uri,
tempFile);
- _logger.info("Downloaded file from {} to {}; Length of downloaded file:
{}", uri, tempFile, tempFile.length());
+ boolean downloadResult = downloadFromURI(uri, tempFile);
Review comment:
Done by adding a new segment fetcher.
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] chenboat commented on a change in pull request #4914: [POC] By-passing deep-store requirement for Realtime segment completion
chenboat commented on a change in pull request #4914: [POC] By-passing
deep-store requirement for Realtime segment completion
URL: https://github.com/apache/incubator-pinot/pull/4914#discussion_r398959989
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeTableDataManager.java
##
@@ -257,21 +280,78 @@ public void downloadAndReplaceSegment(String
segmentName, LLCRealtimeSegmentZKMe
final File segmentFolder = new File(_indexDir, segmentName);
FileUtils.deleteQuietly(segmentFolder);
try {
-
SegmentFetcherFactory.getInstance().getSegmentFetcherBasedOnURI(uri).fetchSegmentToLocal(uri,
tempFile);
- _logger.info("Downloaded file from {} to {}; Length of downloaded file:
{}", uri, tempFile, tempFile.length());
+ boolean downloadResult = downloadFromURI(uri, tempFile);
+ if (!downloadResult) {
+ String peerServerUri = getPeerServerURI(segmentName);
+ if (peerServerUri == null || !downloadFromURI(peerServerUri,
tempFile)) {
+ _logger.warn("Download segment {} from {} failed.", segmentName,
peerServerUri);
+ return;
+ }
+ }
TarGzCompressionUtils.unTar(tempFile, tempSegmentFolder);
- _logger.info("Uncompressed file {} into tmp dir {}", tempFile,
tempSegmentFolder);
+ _logger.warn("Uncompressed file {} into tmp dir {}", tempFile,
tempSegmentFolder);
FileUtils.moveDirectory(tempSegmentFolder.listFiles()[0], segmentFolder);
- _logger.info("Replacing LLC Segment {}", segmentName);
+ _logger.warn("Replacing LLC Segment {}", segmentName);
replaceLLSegment(segmentName, indexLoadingConfig);
} catch (Exception e) {
+ _logger.error("Failed to download segment {}.", e.getMessage());
throw new RuntimeException(e);
} finally {
FileUtils.deleteQuietly(tempFile);
FileUtils.deleteQuietly(tempSegmentFolder);
}
}
+ // Return the address of an ONLINE server hosting a segment.
+ private String getPeerServerURI(String segmentName) {
+ExternalView externalViewForResource =
HelixHelper.getExternalViewForResource(_helixAdmin, _clusterName,
_tableNameWithType);
+if (externalViewForResource == null ) {
+ _logger.warn("External View not found for segment {}", segmentName);
+ return null;
+}
+// Find out the ONLINE server serving the segment.
+for(String segment : externalViewForResource.getPartitionSet()) {
+ if (!segmentName.equals(segment)) {
+continue;
+ }
+
+ Map instanceToStateMap =
externalViewForResource.getStateMap(segmentName);
+ for (Map.Entry instanceState :
instanceToStateMap.entrySet()) {
+if ("ONLINE".equals(instanceState.getValue())) {
+ _logger.info("Found ONLINE server {} for segment {}.",
instanceState.getKey(), segmentName);
+ String instanceId = instanceState.getKey();
+
+ String namePortStr =
instanceId.split(CommonConstants.Helix.PREFIX_OF_SERVER_INSTANCE)[1];
+ String hostName = namePortStr.split("_")[0];
Review comment:
Done.
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] chenboat commented on a change in pull request #4914: [POC] By-passing deep-store requirement for Realtime segment completion
chenboat commented on a change in pull request #4914: [POC] By-passing
deep-store requirement for Realtime segment completion
URL: https://github.com/apache/incubator-pinot/pull/4914#discussion_r388023936
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeTableDataManager.java
##
@@ -257,21 +280,78 @@ public void downloadAndReplaceSegment(String
segmentName, LLCRealtimeSegmentZKMe
final File segmentFolder = new File(_indexDir, segmentName);
FileUtils.deleteQuietly(segmentFolder);
try {
-
SegmentFetcherFactory.getInstance().getSegmentFetcherBasedOnURI(uri).fetchSegmentToLocal(uri,
tempFile);
- _logger.info("Downloaded file from {} to {}; Length of downloaded file:
{}", uri, tempFile, tempFile.length());
+ boolean downloadResult = downloadFromURI(uri, tempFile);
+ if (!downloadResult) {
+ String peerServerUri = getPeerServerURI(segmentName);
+ if (peerServerUri == null || !downloadFromURI(peerServerUri,
tempFile)) {
+ _logger.warn("Download segment {} from {} failed.", segmentName,
peerServerUri);
+ return;
+ }
+ }
TarGzCompressionUtils.unTar(tempFile, tempSegmentFolder);
- _logger.info("Uncompressed file {} into tmp dir {}", tempFile,
tempSegmentFolder);
+ _logger.warn("Uncompressed file {} into tmp dir {}", tempFile,
tempSegmentFolder);
FileUtils.moveDirectory(tempSegmentFolder.listFiles()[0], segmentFolder);
- _logger.info("Replacing LLC Segment {}", segmentName);
+ _logger.warn("Replacing LLC Segment {}", segmentName);
replaceLLSegment(segmentName, indexLoadingConfig);
} catch (Exception e) {
+ _logger.error("Failed to download segment {}.", e.getMessage());
throw new RuntimeException(e);
} finally {
FileUtils.deleteQuietly(tempFile);
FileUtils.deleteQuietly(tempSegmentFolder);
}
}
+ // Return the address of an ONLINE server hosting a segment.
+ private String getPeerServerURI(String segmentName) {
+ExternalView externalViewForResource =
HelixHelper.getExternalViewForResource(_helixAdmin, _clusterName,
_tableNameWithType);
+if (externalViewForResource == null ) {
+ _logger.warn("External View not found for segment {}", segmentName);
+ return null;
+}
+// Find out the ONLINE server serving the segment.
+for(String segment : externalViewForResource.getPartitionSet()) {
+ if (!segmentName.equals(segment)) {
+continue;
+ }
+
+ Map instanceToStateMap =
externalViewForResource.getStateMap(segmentName);
+ for (Map.Entry instanceState :
instanceToStateMap.entrySet()) {
+if ("ONLINE".equals(instanceState.getValue())) {
+ _logger.info("Found ONLINE server {} for segment {}.",
instanceState.getKey(), segmentName);
+ String instanceId = instanceState.getKey();
+
+ String namePortStr =
instanceId.split(CommonConstants.Helix.PREFIX_OF_SERVER_INSTANCE)[1];
+ String hostName = namePortStr.split("_")[0];
+
+ Map instanceConfig =
+ HelixHelper.getInstanceConfigsMapFor(instanceId, _clusterName,
_helixAdmin);
+ int port;
+ try {
+port =
Integer.parseInt(instanceConfig.get(CommonConstants.Helix.Instance.ADMIN_PORT_KEY));
+ } catch (Exception e) {
+port = CommonConstants.Helix.DEFAULT_SERVER_NETTY_PORT;
+ }
+
+ return StringUtil.join("/", "http://; + hostName + ":" + port,
Review comment:
done.
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] chenboat commented on a change in pull request #4914: [POC] By-passing deep-store requirement for Realtime segment completion
chenboat commented on a change in pull request #4914: [POC] By-passing
deep-store requirement for Realtime segment completion
URL: https://github.com/apache/incubator-pinot/pull/4914#discussion_r379704319
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/LLRealtimeSegmentDataManager.java
##
@@ -769,12 +784,21 @@ protected boolean commitSegment(String controllerVipUrl,
boolean isSplitCommit)
if (!segTarFile.exists()) {
throw new RuntimeException("Segment file does not exist:" +
segTarFileName);
}
+// Set the flag to true to prevent the server delete the segment for
upload after receiving OFFLINE->ONLINE
+// message from helix.
+_waitingForUploadToSegmentStore = true;
SegmentCompletionProtocol.Response commitResponse =
commit(controllerVipUrl, isSplitCommit);
if
(!commitResponse.getStatus().equals(SegmentCompletionProtocol.ControllerResponseStatus.COMMIT_SUCCESS))
{
return false;
}
-
+// Asynchronously upload the segment file to Pinot FS for backup. The
upload result does not change the segment
+// completion protocol flow.
+if (!_indexLoadingConfig.isEnableSegmentUploadToController()) {
Review comment:
Done. Check the segment store config too before doing upload.
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] chenboat commented on a change in pull request #4914: [POC] By-passing deep-store requirement for Realtime segment completion
chenboat commented on a change in pull request #4914: [POC] By-passing deep-store requirement for Realtime segment completion URL: https://github.com/apache/incubator-pinot/pull/4914#discussion_r379693686 ## File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/LLCSegmentCompletionHandlers.java ## @@ -396,7 +398,7 @@ public String segmentCommitEndWithMetadata(@QueryParam(SegmentCompletionProtocol requestParams.withInstanceId(instanceId).withSegmentName(segmentName).withOffset(offset) .withSegmentLocation(segmentLocation).withSegmentSizeBytes(segmentSizeBytes) .withBuildTimeMillis(buildTimeMillis).withWaitTimeMillis(waitTimeMillis).withNumRows(numRows) -.withMemoryUsedBytes(memoryUsedBytes); + .withMemoryUsedBytes(memoryUsedBytes).withSegmentUploadToController(segmentUpload); Review comment: Updated the PR. Now in SegmentCompletionFSM.commitSegment(), a new variable segmentFinalLocation is added which can either vip based or the one reported by the server. This string value will then be saved in zk. 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 With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] chenboat commented on a change in pull request #4914: [POC] By-passing deep-store requirement for Realtime segment completion
chenboat commented on a change in pull request #4914: [POC] By-passing
deep-store requirement for Realtime segment completion
URL: https://github.com/apache/incubator-pinot/pull/4914#discussion_r379682967
##
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/CommonConstants.java
##
@@ -197,6 +197,9 @@ public ServerType getServerType() {
public static final String CONFIG_OF_ENABLE_COMMIT_END_WITH_METADATA =
"pinot.server.instance.enable.commitend.metadata";
public static final String CONFIG_OF_REALTIME_OFFHEAP_ALLOCATION =
"pinot.server.instance.realtime.alloc.offheap";
+public static final String CONFIG_OF_REALTIME_ENABLE_UPLOAD_TO_CONTROLLER
= "pinot.server.instance.enable.upload.segment.to.controller";
Review comment:
Comments added.
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] chenboat commented on a change in pull request #4914: [POC] By-passing deep-store requirement for Realtime segment completion
chenboat commented on a change in pull request #4914: [POC] By-passing deep-store requirement for Realtime segment completion URL: https://github.com/apache/incubator-pinot/pull/4914#discussion_r379682423 ## File path: pinot-common/src/main/java/org/apache/pinot/common/protocols/SegmentCompletionProtocol.java ## @@ -123,6 +123,9 @@ public static final String PARAM_MEMORY_USED_BYTES = "memoryUsedBytes"; public static final String PARAM_SEGMENT_SIZE_BYTES = "segmentSizeBytes"; public static final String PARAM_REASON = "reason"; + // Controller whether servers upload segments to controller. Review comment: done. 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 With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] chenboat commented on a change in pull request #4914: [POC] By-passing deep-store requirement for Realtime segment completion
chenboat commented on a change in pull request #4914: [POC] By-passing
deep-store requirement for Realtime segment completion
URL: https://github.com/apache/incubator-pinot/pull/4914#discussion_r379682288
##
File path:
pinot-common/src/main/java/org/apache/pinot/common/protocols/SegmentCompletionProtocol.java
##
@@ -201,6 +205,7 @@ public String getUrl(String hostPort, String protocol) {
private String _segmentLocation;
private long _memoryUsedBytes;
private long _segmentSizeBytes;
+ private boolean _segmentUploadToController;
Review comment:
done.
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] chenboat commented on a change in pull request #4914: [POC] By-passing deep-store requirement for Realtime segment completion
chenboat commented on a change in pull request #4914: [POC] By-passing deep-store requirement for Realtime segment completion URL: https://github.com/apache/incubator-pinot/pull/4914#discussion_r378582257 ## File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/LLCSegmentCompletionHandlers.java ## @@ -396,7 +398,7 @@ public String segmentCommitEndWithMetadata(@QueryParam(SegmentCompletionProtocol requestParams.withInstanceId(instanceId).withSegmentName(segmentName).withOffset(offset) .withSegmentLocation(segmentLocation).withSegmentSizeBytes(segmentSizeBytes) .withBuildTimeMillis(buildTimeMillis).withWaitTimeMillis(waitTimeMillis).withNumRows(numRows) -.withMemoryUsedBytes(memoryUsedBytes); + .withMemoryUsedBytes(memoryUsedBytes).withSegmentUploadToController(segmentUpload); Review comment: Good point. I guess this is related to the change in the updateCommittingSegmentZKMetadata() to: committingSegmentZKMetadata.setDownloadUrl(committingSegmentDescriptor.getSegmentLocation()); In the current code, the Download URL is essentially hardcoded to the vip-base url. With my change, I also need to update the segmentLocation in committingSegmentDescriptor after the segment is moved from tmp location to permanent location during a split-commit. Then the above change should work in all cases. 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 With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] chenboat commented on a change in pull request #4914: [POC] By-passing deep-store requirement for Realtime segment completion
chenboat commented on a change in pull request #4914: [POC] By-passing
deep-store requirement for Realtime segment completion
URL: https://github.com/apache/incubator-pinot/pull/4914#discussion_r378574056
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/SegmentCompletionManager.java
##
@@ -1028,7 +1028,7 @@ 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) {
+ if (isSplitCommit && reqParams.getSegmentUploadToController()) {
Review comment:
Only when the server chooses to upload the segment to controller, there is
still a need for move from tmp location to the final location. This is why I
add an additional checking here.
If the server uploads the segment to deep store directly, there is no need
to move anymore.
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] chenboat commented on a change in pull request #4914: [POC] By-passing deep-store requirement for Realtime segment completion
chenboat commented on a change in pull request #4914: [POC] By-passing deep-store requirement for Realtime segment completion URL: https://github.com/apache/incubator-pinot/pull/4914#discussion_r378470584 ## File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/LLCSegmentCompletionHandlers.java ## @@ -396,7 +398,7 @@ public String segmentCommitEndWithMetadata(@QueryParam(SegmentCompletionProtocol requestParams.withInstanceId(instanceId).withSegmentName(segmentName).withOffset(offset) .withSegmentLocation(segmentLocation).withSegmentSizeBytes(segmentSizeBytes) .withBuildTimeMillis(buildTimeMillis).withWaitTimeMillis(waitTimeMillis).withNumRows(numRows) -.withMemoryUsedBytes(memoryUsedBytes); + .withMemoryUsedBytes(memoryUsedBytes).withSegmentUploadToController(segmentUpload); Review comment: The getUrl() of the SegmentCompletionProtocol.Request method has been updated to include the new parameter (i.e., _segmentUploadToController). The default value is true. So my understanding is that the URL construction for Pinot servers have been taken care of. Am I missing anything? 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 With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] chenboat commented on a change in pull request #4914: [POC] By-passing deep-store requirement for Realtime segment completion
chenboat commented on a change in pull request #4914: [POC] By-passing deep-store requirement for Realtime segment completion URL: https://github.com/apache/incubator-pinot/pull/4914#discussion_r378470584 ## File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/LLCSegmentCompletionHandlers.java ## @@ -396,7 +398,7 @@ public String segmentCommitEndWithMetadata(@QueryParam(SegmentCompletionProtocol requestParams.withInstanceId(instanceId).withSegmentName(segmentName).withOffset(offset) .withSegmentLocation(segmentLocation).withSegmentSizeBytes(segmentSizeBytes) .withBuildTimeMillis(buildTimeMillis).withWaitTimeMillis(waitTimeMillis).withNumRows(numRows) -.withMemoryUsedBytes(memoryUsedBytes); + .withMemoryUsedBytes(memoryUsedBytes).withSegmentUploadToController(segmentUpload); Review comment: The getUrl() of the SegmentCompletionProtocol.Request method has been updated to include the new parameter (i.e., _segmentUploadToController). The default value is true. So my understanding is that the URL construction for Pinot servers have been taken care of. Am I missing anything? 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 With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] chenboat commented on a change in pull request #4914: [POC] By-passing deep-store requirement for Realtime segment completion
chenboat commented on a change in pull request #4914: [POC] By-passing
deep-store requirement for Realtime segment completion
URL: https://github.com/apache/incubator-pinot/pull/4914#discussion_r360610944
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeTableDataManager.java
##
@@ -257,21 +280,78 @@ public void downloadAndReplaceSegment(String
segmentName, LLCRealtimeSegmentZKMe
final File segmentFolder = new File(_indexDir, segmentName);
FileUtils.deleteQuietly(segmentFolder);
try {
-
SegmentFetcherFactory.getInstance().getSegmentFetcherBasedOnURI(uri).fetchSegmentToLocal(uri,
tempFile);
- _logger.info("Downloaded file from {} to {}; Length of downloaded file:
{}", uri, tempFile, tempFile.length());
+ boolean downloadResult = downloadFromURI(uri, tempFile);
+ if (!downloadResult) {
+ String peerServerUri = getPeerServerURI(segmentName);
+ if (peerServerUri == null || !downloadFromURI(peerServerUri,
tempFile)) {
+ _logger.warn("Download segment {} from {} failed.", segmentName,
peerServerUri);
+ return;
+ }
+ }
TarGzCompressionUtils.unTar(tempFile, tempSegmentFolder);
- _logger.info("Uncompressed file {} into tmp dir {}", tempFile,
tempSegmentFolder);
+ _logger.warn("Uncompressed file {} into tmp dir {}", tempFile,
tempSegmentFolder);
Review comment:
should be a INFO-- it is a leftover when I tried to verify the behavior
during tests.
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] chenboat commented on a change in pull request #4914: [POC] By-passing deep-store requirement for Realtime segment completion
chenboat commented on a change in pull request #4914: [POC] By-passing
deep-store requirement for Realtime segment completion
URL: https://github.com/apache/incubator-pinot/pull/4914#discussion_r360610807
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/LLRealtimeSegmentDataManager.java
##
@@ -769,12 +784,21 @@ protected boolean commitSegment(String controllerVipUrl,
boolean isSplitCommit)
if (!segTarFile.exists()) {
throw new RuntimeException("Segment file does not exist:" +
segTarFileName);
}
+// Set the flag to true to prevent the server delete the segment for
upload after receiving OFFLINE->ONLINE
+// message from helix.
+_waitingForUploadToSegmentStore = true;
SegmentCompletionProtocol.Response commitResponse =
commit(controllerVipUrl, isSplitCommit);
if
(!commitResponse.getStatus().equals(SegmentCompletionProtocol.ControllerResponseStatus.COMMIT_SUCCESS))
{
return false;
}
-
+// Asynchronously upload the segment file to Pinot FS for backup. The
upload result does not change the segment
+// completion protocol flow.
+if (!_indexLoadingConfig.isEnableSegmentUploadToController()) {
Review comment:
Yes. I should introduced one more flag to control the behavior. Right now my
logic is that if server does not upload the segment to controller, it must
upload the segment to deep storage -- which might not always be the case if
there is no deep storage configured.
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] chenboat commented on a change in pull request #4914: [POC] By-passing deep-store requirement for Realtime segment completion
chenboat commented on a change in pull request #4914: [POC] By-passing
deep-store requirement for Realtime segment completion
URL: https://github.com/apache/incubator-pinot/pull/4914#discussion_r360610172
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/LLRealtimeSegmentDataManager.java
##
@@ -262,6 +264,8 @@ public void deleteSegmentFile() {
private final boolean _isOffHeap;
private final boolean _nullHandlingEnabled;
private final SegmentCommitterFactory _segmentCommitterFactory;
+ // Indicating if the segment is waiting to be upload to segment stores.
+ private boolean _waitingForUploadToSegmentStore = false;
Review comment:
It is set to be true in line 789 when the committer still needs to upload
the segment to deep store.
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] chenboat commented on a change in pull request #4914: [POC] By-passing deep-store requirement for Realtime segment completion
chenboat commented on a change in pull request #4914: [POC] By-passing
deep-store requirement for Realtime segment completion
URL: https://github.com/apache/incubator-pinot/pull/4914#discussion_r360609559
##
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/CommonConstants.java
##
@@ -197,6 +197,9 @@ public ServerType getServerType() {
public static final String CONFIG_OF_ENABLE_COMMIT_END_WITH_METADATA =
"pinot.server.instance.enable.commitend.metadata";
public static final String CONFIG_OF_REALTIME_OFFHEAP_ALLOCATION =
"pinot.server.instance.realtime.alloc.offheap";
+public static final String CONFIG_OF_REALTIME_ENABLE_UPLOAD_TO_CONTROLLER
= "pinot.server.instance.enable.upload.segment.to.controller";
Review comment:
This parameter controls if a committer server uploads segments to the
controller or not. It should be named to enableSegmentUploadToController(). If
it is not enabled, the server will only upload meta data the controller.
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] chenboat commented on a change in pull request #4914: [POC] By-passing deep-store requirement for Realtime segment completion
chenboat commented on a change in pull request #4914: [POC] By-passing
deep-store requirement for Realtime segment completion
URL: https://github.com/apache/incubator-pinot/pull/4914#discussion_r360609441
##
File path:
pinot-common/src/main/java/org/apache/pinot/common/protocols/SegmentCompletionProtocol.java
##
@@ -186,7 +189,8 @@ public String getUrl(String hostPort, String protocol) {
+ (_params.getSegmentSizeBytes() <= 0 ? ""
: ("&" + PARAM_SEGMENT_SIZE_BYTES + "=" +
_params.getSegmentSizeBytes())) + (_params.getNumRows() <= 0 ? ""
: ("&" + PARAM_ROW_COUNT + "=" + _params.getNumRows())) +
(_params.getSegmentLocation() == null ? ""
- : ("&" + PARAM_SEGMENT_LOCATION + "=" +
_params.getSegmentLocation()));
+ : ("&" + PARAM_SEGMENT_LOCATION + "=" +
_params.getSegmentLocation()))
+ + "&" + PARAM_SEGMENT_UPLOAD_TO_CONTROLLER + "=" +
_params.getSegmentUploadToController();
Review comment:
This parameter controls if a committer server uploads segments to the
controller or not. It should be named to enableSegmentUploadToController().
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
