Jackie-Jiang commented on code in PR #9624:
URL: https://github.com/apache/pinot/pull/9624#discussion_r998800111
##########
pinot-controller/src/main/java/org/apache/pinot/controller/ControllerConf.java:
##########
@@ -184,6 +184,9 @@ public static class ControllerPeriodicTasksConf {
"controller.realtimeSegmentRelocation.initialDelayInSeconds";
public static final String SEGMENT_RELOCATOR_INITIAL_DELAY_IN_SECONDS =
"controller.segmentRelocator.initialDelayInSeconds";
+ public static final String SEGMENT_RELOCATOR_ENABLE_LOCAL_TIER_MIGRATION =
Review Comment:
Why do we need a separate config for this? Isn't this the same as tier
assigner?
##########
pinot-common/src/main/java/org/apache/pinot/common/messages/SegmentReloadMessage.java:
##########
@@ -34,19 +35,25 @@ public class SegmentReloadMessage extends Message {
public static final String RELOAD_SEGMENT_MSG_SUB_TYPE = "RELOAD_SEGMENT";
private static final String FORCE_DOWNLOAD_KEY = "forceDownload";
+ private static final String SEGMENT_LIST_KEY = "segmentList";
- public SegmentReloadMessage(@Nonnull String tableNameWithType, @Nullable
String segmentName, boolean forceDownload) {
+ public SegmentReloadMessage(@Nonnull String tableNameWithType, @Nullable
List<String> segmentNames,
+ boolean forceDownload) {
super(MessageType.USER_DEFINE_MSG, UUID.randomUUID().toString());
setResourceName(tableNameWithType);
- if (segmentName != null) {
- setPartitionName(segmentName);
- }
setMsgSubType(RELOAD_SEGMENT_MSG_SUB_TYPE);
// Give it infinite time to process the message, as long as session is
alive
setExecutionTimeout(-1);
ZNRecord znRecord = getRecord();
znRecord.setBooleanField(FORCE_DOWNLOAD_KEY, forceDownload);
+ if (segmentNames != null) {
+ if (segmentNames.size() == 1) {
+ setPartitionName(segmentNames.get(0));
+ } else {
+ znRecord.setListField(SEGMENT_LIST_KEY, segmentNames);
Review Comment:
Let's always set the `SEGMENT_LIST_KEY` regardless of the number of
segments. In the future we can always read the list field. We need to also set
the partition name for single segment for backward compatibility. Let's also
add a TODO to remove it after the next release
##########
pinot-common/src/main/java/org/apache/pinot/common/messages/SegmentReloadMessage.java:
##########
@@ -59,4 +66,8 @@ public SegmentReloadMessage(Message message) {
public boolean shouldForceDownload() {
return getRecord().getBooleanField(FORCE_DOWNLOAD_KEY, false);
}
+
+ public List<String> getSegmentList() {
Review Comment:
Annotate it as `@Nullable`
##########
pinot-common/src/main/java/org/apache/pinot/common/messages/SegmentReloadMessage.java:
##########
@@ -34,19 +35,25 @@ public class SegmentReloadMessage extends Message {
public static final String RELOAD_SEGMENT_MSG_SUB_TYPE = "RELOAD_SEGMENT";
private static final String FORCE_DOWNLOAD_KEY = "forceDownload";
+ private static final String SEGMENT_LIST_KEY = "segmentList";
- public SegmentReloadMessage(@Nonnull String tableNameWithType, @Nullable
String segmentName, boolean forceDownload) {
+ public SegmentReloadMessage(@Nonnull String tableNameWithType, @Nullable
List<String> segmentNames,
Review Comment:
(minor, convention) Let's remove the `@Nonnull` annotation as it is very
easy to misread as `@Nullable`
##########
pinot-common/src/main/java/org/apache/pinot/common/messages/SegmentReloadMessage.java:
##########
@@ -34,19 +35,25 @@ public class SegmentReloadMessage extends Message {
public static final String RELOAD_SEGMENT_MSG_SUB_TYPE = "RELOAD_SEGMENT";
private static final String FORCE_DOWNLOAD_KEY = "forceDownload";
+ private static final String SEGMENT_LIST_KEY = "segmentList";
Review Comment:
(minor) Seems we are using `SEGMENT_NAMES` key in `ForceCommitMessage`,
consider using the same key (but store as list field because that seems
preferred)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
