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 2021/03/15 17:51:44 UTC

[GitHub] [incubator-pinot] snleee commented on a change in pull request #6682: [WIP]Fix bug #6671: RealtimeTableDataManager shuts down SegmentBuildTimeLeaseExtender for all tables in the host

snleee commented on a change in pull request #6682:
URL: https://github.com/apache/incubator-pinot/pull/6682#discussion_r594550502



##########
File path: pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/SegmentBuildTimeLeaseExtender.java
##########
@@ -46,25 +46,25 @@
   // Retransmit lease request 10% before lease expires.
   private static final int REPEAT_REQUEST_PERIOD_SEC = (EXTRA_TIME_SECONDS * 9 / 10);
   private static Logger LOGGER = LoggerFactory.getLogger(SegmentBuildTimeLeaseExtender.class);
-  private static final Map<String, SegmentBuildTimeLeaseExtender> INSTANCE_TO_LEASE_EXTENDER = new HashMap<>(1);
+  private static final Map<String, SegmentBuildTimeLeaseExtender> TABLE_TO_LEASE_EXTENDER = new HashMap<>(1);

Review comment:
       Can you change `new HashMap<>(1) to new HashMap<>()`? I think that we specifically built the hashmap with the size 1 because we only kept 1 lease extender per server while we now keep 1 lease extender per table. 

##########
File path: pinot-core/src/test/java/org/apache/pinot/core/data/manager/realtime/LLRealtimeSegmentDataManagerTest.java
##########
@@ -128,9 +131,9 @@ private TableConfig createTableConfig()
     return JsonUtils.stringToObject(_tableConfigJson, TableConfig.class);
   }
 
-  private RealtimeTableDataManager createTableDataManager() {
+  private RealtimeTableDataManager createTableDataManager(TableConfig tableConfig) {

Review comment:
       Can you add the test for the particular issue that we had if possible?
   
   1. Create 2 data managers for 2 tables.
   2. Call the shutdown() from one data manager.
   3. Check if the lease extender for 2nd data manager is still alive. (Also double check if this test would fail before the fix)




----------------------------------------------------------------
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:
users@infra.apache.org



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