You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2022/05/04 23:19:10 UTC

[GitHub] [pinot] GSharayu opened a new pull request, #8642: Pre-req work for SegmentAssignmentStrategy (#8585)

GSharayu opened a new pull request, #8642:
URL: https://github.com/apache/pinot/pull/8642

   Currently, we have SegmentAssignment interface and OfflineSegmentAssignment & RealtimeSegmentAssignment as its implementation. This approach doesn't allow us to pick the custom segment assignment strategy.
   
   This PR provides SegmentAssignmentStrategy as an interface improvement and pre-requisite work. 
   
   Issue #8585 


-- 
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-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] GSharayu closed pull request #8642: [WIP][Not Ready for Review]Pre-req work for SegmentAssignmentStrategy (#8585)

Posted by GitBox <gi...@apache.org>.
GSharayu closed pull request #8642: [WIP][Not Ready for Review]Pre-req work for SegmentAssignmentStrategy (#8585)
URL: https://github.com/apache/pinot/pull/8642


-- 
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-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] snleee commented on pull request #8642: Pre-req work for SegmentAssignmentStrategy (#8585)

Posted by GitBox <gi...@apache.org>.
snleee commented on PR #8642:
URL: https://github.com/apache/pinot/pull/8642#issuecomment-1118029484

   So Ideally, I think that the following structure is needed:
   
   ```
   1. SegmentAssignementStrategy -> ReplicaGroupSegmentAssignmentStrategy, BalancedSegmentAssignmentStrategy, CostBasedSegmentAssignmentStrategy etc
   2. SegmentAssignment -> OfflineSegmentAssignment, RealtimeSegmentAssignment, etc
   ```
   
   `Offline/RealtimeSegmentAssignment` implementation should pick replica group vs balanced based on the offline table config.
   


-- 
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-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] siddharthteotia commented on a diff in pull request #8642: Pre-req work for SegmentAssignmentStrategy (#8585)

Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on code in PR #8642:
URL: https://github.com/apache/pinot/pull/8642#discussion_r865490430


##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/OfflineSegmentAssignmentStrategy.java:
##########
@@ -68,8 +68,8 @@
  *   </li>
  * </ul>
  */
-public class OfflineSegmentAssignment implements SegmentAssignment {
-  private static final Logger LOGGER = LoggerFactory.getLogger(OfflineSegmentAssignment.class);
+public class OfflineSegmentAssignmentStrategy implements SegmentAssignmentStrategy {

Review Comment:
   This does not seem correct to me
   
   OfflineSegmentAssignment or RealtimeSegmentAssignment is not a strategy imo
   
   Regardless of offline v/s realtime, replica group and balanced are the concrete implementations of strategies
   
   FYI @jasperjiaguo  since you are also going to touch related code later



##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/OfflineSegmentAssignmentStrategy.java:
##########
@@ -68,8 +68,8 @@
  *   </li>
  * </ul>
  */
-public class OfflineSegmentAssignment implements SegmentAssignment {
-  private static final Logger LOGGER = LoggerFactory.getLogger(OfflineSegmentAssignment.class);
+public class OfflineSegmentAssignmentStrategy implements SegmentAssignmentStrategy {

Review Comment:
   This does not seem correct to me
   
   OfflineSegmentAssignment or RealtimeSegmentAssignment is not a strategy imo
   
   Regardless of offline v/s realtime, replica group and balanced are the concrete implementations of strategies
   
   cc @jasperjiaguo  since you are also going to touch related code later



-- 
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-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] jackjlli commented on a diff in pull request #8642: Pre-req work for SegmentAssignmentStrategy (#8585)

Posted by GitBox <gi...@apache.org>.
jackjlli commented on code in PR #8642:
URL: https://github.com/apache/pinot/pull/8642#discussion_r865484444


##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/SegmentAssignmentFactory.java:
##########
@@ -24,21 +24,29 @@
 
 
 /**
- * Factory for the {@link SegmentAssignment}.
+ * Factory for the {@link SegmentAssignmentStrategy}.
  */
 public class SegmentAssignmentFactory {

Review Comment:
   sed 's/SegmentAssignmentFactory/SegmentAssignmentStrategyFactory/'



-- 
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-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] Jackie-Jiang commented on pull request #8642: [WIP][Not Ready for Review]Pre-req work for SegmentAssignmentStrategy (#8585)

Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on PR #8642:
URL: https://github.com/apache/pinot/pull/8642#issuecomment-1118948143

   I don't see the difference of the new `SegmentAssignmentStrategy` and the existing `SegmentAssignment`. If you want to plug-in custom assignment, just add the support to `SegmentAssignmentFactory`


-- 
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-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] GSharayu commented on pull request #8642: Pre-req work for SegmentAssignmentStrategy (#8585)

Posted by GitBox <gi...@apache.org>.
GSharayu commented on PR #8642:
URL: https://github.com/apache/pinot/pull/8642#issuecomment-1118020139

   @snleee @sajjad-moradi 


-- 
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-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org