Jackie-Jiang commented on a change in pull request #7329:
URL: https://github.com/apache/pinot/pull/7329#discussion_r692505742



##########
File path: 
pinot-common/src/main/java/org/apache/pinot/common/lineage/SegmentLineageAccessHelper.java
##########
@@ -79,4 +80,17 @@ public static boolean 
writeSegmentLineage(ZkHelixPropertyStore<ZNRecord> propert
     String path = 
ZKMetadataProvider.constructPropertyStorePathForSegmentLineage(tableNameWithType);
     return propertyStore.set(path, segmentLineage.toZNRecord(), 
expectedVersion, AccessOption.PERSISTENT);
   }
+
+  /**
+   * Delete the segment lineage from the property store
+   *
+   * @param propertyStore a property store
+   * @param tableNameWithType a table name with type
+   */
+  public static void deleteSegmentLineage(ZkHelixPropertyStore<ZNRecord> 
propertyStore, String tableNameWithType) {

Review comment:
       Same here, let's return boolean

##########
File path: 
pinot-common/src/main/java/org/apache/pinot/common/minion/MinionTaskMetadataUtils.java
##########
@@ -51,44 +50,46 @@ public static ZNRecord 
fetchMinionTaskMetadataZNRecord(HelixPropertyStore<ZNReco
     return znRecord;
   }
 
+  /**
+   * Deletes the ZNRecord for the given minion task and tableName, from 
MINION_TASK_METADATA/taskName/tableNameWthType
+   */
+  public static void deleteTaskMetadata(HelixPropertyStore<ZNRecord> 
propertyStore, String taskType,

Review comment:
       Return boolean

##########
File path: 
pinot-common/src/main/java/org/apache/pinot/common/assignment/InstancePartitionsUtils.java
##########
@@ -142,9 +142,10 @@ public static void 
persistInstancePartitions(HelixPropertyStore<ZNRecord> proper
   /**
    * Removes the instance partitions from Helix property store.
    */
-  public static void removeInstancePartitions(HelixPropertyStore<ZNRecord> 
propertyStore, String instancePartitionsName) {
+  public static void removeInstancePartitions(HelixPropertyStore<ZNRecord> 
propertyStore,

Review comment:
       Let's return a boolean instead so that the caller can handle the return 
properly, e.g. the rest API might want to throw exception when the instance 
partitions do not exist




-- 
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

Reply via email to