[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #9212: Add close method to upsert interfaces

[GitBox <gi...@apache.org>]

Jackie-Jiang commented on code in PR #9212:
URL: https://github.com/apache/pinot/pull/9212#discussion_r946224636


##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/TableUpsertMetadataManager.java:
##########
@@ -37,4 +37,6 @@ public interface TableUpsertMetadataManager {
   ConcurrentMapPartitionUpsertMetadataManager getOrCreatePartitionManager(int partitionId);
 
   UpsertConfig.Mode getUpsertMode();
+
+  void close();

Review Comment:
   Let's make the interface extends `Closeable`



##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/ConcurrentMapPartitionUpsertMetadataManager.java:
##########
@@ -428,6 +428,13 @@ public GenericRow updateRecord(GenericRow record, RecordInfo recordInfo) {
     }
   }
 
+  @Override
+  public void close() {
+    _logger.info("Closing metadata manager for table {} and partition {}, current primary key count: {}",
+        _tableNameWithType, _partitionId, _primaryKeyToRecordLocationMap.size());
+    _primaryKeyToRecordLocationMap.clear();

Review Comment:
   This can be expensive. We may leave it as no-op



##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeTableDataManager.java:
##########
@@ -199,6 +199,10 @@ protected void doStart() {
   @Override
   protected void doShutdown() {
     _segmentAsyncExecutorService.shutdown();
+    if (_tableUpsertMetadataManager != null) {
+      _tableUpsertMetadataManager.close();
+    }
+

Review Comment:
   (nit) remove this empty line



##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/PartitionUpsertMetadataManager.java:
##########
@@ -82,4 +82,9 @@ public interface PartitionUpsertMetadataManager {
    * Returns the merged record when partial-upsert is enabled.
    */
   GenericRow updateRecord(GenericRow record, RecordInfo recordInfo);
+
+  /**
+   * Close the partition upsert data manager
+   */
+  void close();

Review Comment:
   Let's make the interface extends `Closeable`



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