Re: [PR] Add instance level consumer dir usage metric [pinot]
jasperjiaguo merged PR #14430: URL: https://github.com/apache/pinot/pull/14430 -- 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
Re: [PR] Add instance level consumer dir usage metric [pinot]
SabrinaZhaozyf commented on code in PR #14430: URL: https://github.com/apache/pinot/pull/14430#discussion_r1840938825 ## pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeTableDataManager.java: ## @@ -102,7 +102,7 @@ public class RealtimeTableDataManager extends BaseTableDataManager { // The old name of the stats file used to be stats.ser which we changed when we moved all packages // from com.linkedin to org.apache because of not being able to deserialize the old files using the newer classes private static final String STATS_FILE_NAME = "segment-stats.ser"; - private static final String CONSUMERS_DIR = "consumers"; + public static final String CONSUMERS_DIR = "consumers"; Review Comment: done ## pinot-common/src/main/java/org/apache/pinot/common/metrics/ServerGauge.java: ## @@ -76,7 +76,8 @@ public enum ServerGauge implements AbstractMetrics.Gauge { // Needed to track if valid doc id snapshots are present for faster restarts UPSERT_VALID_DOC_ID_SNAPSHOT_COUNT("upsertValidDocIdSnapshotCount", false), UPSERT_PRIMARY_KEYS_IN_SNAPSHOT_COUNT("upsertPrimaryKeysInSnapshotCount", false), - REALTIME_INGESTION_OFFSET_LAG("offsetLag", false); + REALTIME_INGESTION_OFFSET_LAG("offsetLag", false), + CONSUMER_DIR_USAGE("bytes", true); Review Comment: done -- 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
Re: [PR] Add instance level consumer dir usage metric [pinot]
vvivekiyer commented on code in PR #14430: URL: https://github.com/apache/pinot/pull/14430#discussion_r1839194969 ## pinot-common/src/main/java/org/apache/pinot/common/metrics/ServerGauge.java: ## @@ -76,7 +76,8 @@ public enum ServerGauge implements AbstractMetrics.Gauge { // Needed to track if valid doc id snapshots are present for faster restarts UPSERT_VALID_DOC_ID_SNAPSHOT_COUNT("upsertValidDocIdSnapshotCount", false), UPSERT_PRIMARY_KEYS_IN_SNAPSHOT_COUNT("upsertPrimaryKeysInSnapshotCount", false), - REALTIME_INGESTION_OFFSET_LAG("offsetLag", false); + REALTIME_INGESTION_OFFSET_LAG("offsetLag", false), + CONSUMER_DIR_USAGE("bytes", true); Review Comment: (nit) can we name it as `REALTIME_CONSUMER_DIR_USAGE`? ## pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeTableDataManager.java: ## @@ -102,7 +102,7 @@ public class RealtimeTableDataManager extends BaseTableDataManager { // The old name of the stats file used to be stats.ser which we changed when we moved all packages // from com.linkedin to org.apache because of not being able to deserialize the old files using the newer classes private static final String STATS_FILE_NAME = "segment-stats.ser"; - private static final String CONSUMERS_DIR = "consumers"; + public static final String CONSUMERS_DIR = "consumers"; Review Comment: Can we revert this to `private` now? -- 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
Re: [PR] Add instance level consumer dir usage metric [pinot]
SabrinaZhaozyf commented on code in PR #14430: URL: https://github.com/apache/pinot/pull/14430#discussion_r1839096908 ## pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManager.java: ## @@ -180,6 +181,24 @@ private void initInstanceDataDir(File instanceDataDir) { } } + @Override + public List getConsumerDirPaths() { +String consumerDirPath = _instanceDataManagerConfig.getConsumerDir(); + +if (consumerDirPath != null) { + return Collections.singletonList(new File(consumerDirPath)); +} else { + List consumerDirs = new ArrayList<>(); + String instanceDataDir = _instanceDataManagerConfig.getInstanceDataDir(); + for (String tableNameWithType : getAllTables()) { +String dirPath = instanceDataDir + File.separator + tableNameWithType + File.separator ++ RealtimeTableDataManager.CONSUMERS_DIR; +consumerDirs.add(new File(dirPath)); + } + return consumerDirs; Review Comment: When consumerDir is not specified, `RealtimeTableDataManager` constructs a consumer directory under table data directory (`//consumers`). Else case here is to cover that case. -- 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
Re: [PR] Add instance level consumer dir usage metric [pinot]
SabrinaZhaozyf commented on code in PR #14430: URL: https://github.com/apache/pinot/pull/14430#discussion_r1839102597 ## pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManager.java: ## @@ -180,6 +181,24 @@ private void initInstanceDataDir(File instanceDataDir) { } } + @Override + public List getConsumerDirPaths() { +String consumerDirPath = _instanceDataManagerConfig.getConsumerDir(); + +if (consumerDirPath != null) { + return Collections.singletonList(new File(consumerDirPath)); +} else { + List consumerDirs = new ArrayList<>(); + String instanceDataDir = _instanceDataManagerConfig.getInstanceDataDir(); + for (String tableNameWithType : getAllTables()) { +String dirPath = instanceDataDir + File.separator + tableNameWithType + File.separator Review Comment: It will not count offline table dataDir size as the directory will not exist. Latest commit explicitly checks instance type tho, which should make it clearer. -- 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
Re: [PR] Add instance level consumer dir usage metric [pinot]
SabrinaZhaozyf commented on code in PR #14430: URL: https://github.com/apache/pinot/pull/14430#discussion_r1839101674 ## pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManager.java: ## @@ -180,6 +181,24 @@ private void initInstanceDataDir(File instanceDataDir) { } } + @Override + public List getConsumerDirPaths() { +String consumerDirPath = _instanceDataManagerConfig.getConsumerDir(); + +if (consumerDirPath != null) { + return Collections.singletonList(new File(consumerDirPath)); +} else { + List consumerDirs = new ArrayList<>(); Review Comment: done -- 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
Re: [PR] Add instance level consumer dir usage metric [pinot]
SabrinaZhaozyf commented on code in PR #14430: URL: https://github.com/apache/pinot/pull/14430#discussion_r1839101507 ## pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManager.java: ## @@ -180,6 +181,24 @@ private void initInstanceDataDir(File instanceDataDir) { } } + @Override + public List getConsumerDirPaths() { +String consumerDirPath = _instanceDataManagerConfig.getConsumerDir(); + +if (consumerDirPath != null) { + return Collections.singletonList(new File(consumerDirPath)); +} else { Review Comment: updated to use `RealtimeTableDataManager` for this logic which has a comment about this. -- 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
Re: [PR] Add instance level consumer dir usage metric [pinot]
SabrinaZhaozyf commented on code in PR #14430: URL: https://github.com/apache/pinot/pull/14430#discussion_r1839099916 ## pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManager.java: ## @@ -180,6 +181,24 @@ private void initInstanceDataDir(File instanceDataDir) { } } + @Override + public List getConsumerDirPaths() { +String consumerDirPath = _instanceDataManagerConfig.getConsumerDir(); + +if (consumerDirPath != null) { + return Collections.singletonList(new File(consumerDirPath)); +} else { + List consumerDirs = new ArrayList<>(); + String instanceDataDir = _instanceDataManagerConfig.getInstanceDataDir(); + for (String tableNameWithType : getAllTables()) { +String dirPath = instanceDataDir + File.separator + tableNameWithType + File.separator ++ RealtimeTableDataManager.CONSUMERS_DIR; +consumerDirs.add(new File(dirPath)); + } + return consumerDirs; Review Comment: my concern with that is that the stored consumerDirs will need to be updated with table changes on the instance. Thus, relying on table data managers to fetch paths to avoid that. -- 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
Re: [PR] Add instance level consumer dir usage metric [pinot]
SabrinaZhaozyf commented on code in PR #14430: URL: https://github.com/apache/pinot/pull/14430#discussion_r1839098791 ## pinot-server/src/main/java/org/apache/pinot/server/starter/helix/BaseServerStarter.java: ## @@ -707,6 +711,23 @@ public void start() _serverQueriesDisabledTracker = new ServerQueriesDisabledTracker(_helixClusterName, _instanceId, _helixManager, serverMetrics); _serverQueriesDisabledTracker.start(); + +// Add metrics for consumer directory usage +serverMetrics.setOrUpdateGlobalGauge(ServerGauge.CONSUMER_DIR_USAGE, () -> { + List serverConsumerDirs = instanceDataManager.getConsumerDirPaths(); + final long[] totalSize = {0}; Review Comment: Changed to long. Previously used another lambda there that used the array! -- 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
Re: [PR] Add instance level consumer dir usage metric [pinot]
jasperjiaguo commented on code in PR #14430: URL: https://github.com/apache/pinot/pull/14430#discussion_r1838620339 ## pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManager.java: ## @@ -180,6 +181,24 @@ private void initInstanceDataDir(File instanceDataDir) { } } + @Override + public List getConsumerDirPaths() { +String consumerDirPath = _instanceDataManagerConfig.getConsumerDir(); + +if (consumerDirPath != null) { + return Collections.singletonList(new File(consumerDirPath)); +} else { + List consumerDirs = new ArrayList<>(); + String instanceDataDir = _instanceDataManagerConfig.getInstanceDataDir(); + for (String tableNameWithType : getAllTables()) { +String dirPath = instanceDataDir + File.separator + tableNameWithType + File.separator ++ RealtimeTableDataManager.CONSUMERS_DIR; +consumerDirs.add(new File(dirPath)); + } + return consumerDirs; Review Comment: should we store return value (consumerDirs) up so that we don't calculate these every time? We can also add logging to it if it is calculated just once. -- 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
Re: [PR] Add instance level consumer dir usage metric [pinot]
vvivekiyer commented on code in PR #14430: URL: https://github.com/apache/pinot/pull/14430#discussion_r1838747910 ## pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManager.java: ## @@ -180,6 +181,24 @@ private void initInstanceDataDir(File instanceDataDir) { } } + @Override + public List getConsumerDirPaths() { +String consumerDirPath = _instanceDataManagerConfig.getConsumerDir(); + +if (consumerDirPath != null) { + return Collections.singletonList(new File(consumerDirPath)); +} else { + List consumerDirs = new ArrayList<>(); + String instanceDataDir = _instanceDataManagerConfig.getInstanceDataDir(); + for (String tableNameWithType : getAllTables()) { +String dirPath = instanceDataDir + File.separator + tableNameWithType + File.separator Review Comment: Will this also end up considering offline tables? Do we want to just do this for realtime tables ? If we are doing this for all tables, then it might just be enough to calculate the size of instanceDataDir? ## pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManager.java: ## @@ -180,6 +181,24 @@ private void initInstanceDataDir(File instanceDataDir) { } } + @Override + public List getConsumerDirPaths() { +String consumerDirPath = _instanceDataManagerConfig.getConsumerDir(); + +if (consumerDirPath != null) { + return Collections.singletonList(new File(consumerDirPath)); +} else { + List consumerDirs = new ArrayList<>(); Review Comment: This code block is repeating the logic in RealtimeTableDataManager.getConsumerDir right? Can we move that logic to a common place and reuse from both places? Duplicating logic can result in the metrics breaking if this logic changes in`RealtimeTableDataManager.getConsumerDir` -- 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
Re: [PR] Add instance level consumer dir usage metric [pinot]
codecov-commenter commented on PR #14430: URL: https://github.com/apache/pinot/pull/14430#issuecomment-2471353767 ## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/14430?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report All modified and coverable lines are covered by tests :white_check_mark: > Project coverage is 0.00%. Comparing base [(`59551e4`)](https://app.codecov.io/gh/apache/pinot/commit/59551e45224f1535c4863fd577622b37366ccc97?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`b1d36e1`)](https://app.codecov.io/gh/apache/pinot/commit/b1d36e1d39f49b4b856aa52fba9862af44b12a1d?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache). > Report is 1313 commits behind head on master. > :exclamation: There is a different number of reports uploaded between BASE (59551e4) and HEAD (b1d36e1). Click for more details. > > HEAD has 53 uploads less than BASE > >| Flag | BASE (59551e4) | HEAD (b1d36e1) | >|--|--|--| >|integration|7|1| >|integration2|3|1| >|temurin|12|1| >|java-21|7|1| >|skip-bytebuffers-true|3|1| >|skip-bytebuffers-false|7|0| >|unittests|5|0| >|unittests1|2|0| >|java-11|5|0| >|unittests2|3|0| >|integration1|2|0| >|custom-integration1|2|0| > Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #14430 +/- ## = - Coverage 61.75%0.00% -61.76% = Files 24363 -2433 Lines1332336 -133227 Branches 206360-20636 = - Hits 822740-82274 + Misses449116-44905 + Partials 60480 -6048 ``` | [Flag](https://app.codecov.io/gh/apache/pinot/pull/14430/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | | |---|---|---| | [custom-integration1](https://app.codecov.io/gh/apache/pinot/pull/14430/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | | | [integration](https://app.codecov.io/gh/apache/pinot/pull/14430/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <ø> (-0.01%)` | :arrow_down: | | [integration1](https://app.codecov.io/gh/apache/pinot/pull/14430/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | | | [integration2](https://app.codecov.io/gh/apache/pinot/pull/14430/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <ø> (ø)` | | | [java-11](https://app.codecov.io/gh/apache/pinot/pull/14430/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | | | [java-21](https://app.codecov.io/gh/apache/pinot/pull/14430/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <ø> (-61.63%)` | :arrow_down: | | [skip-bytebuffers-false](https://app.codecov.io/gh/apache/pinot/pull/14430/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | | | [skip-bytebuffers-true](https://app.codecov.io/gh/apache/pinot/pull/14430/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <ø> (-27.73%)` | :arrow_down: | | [temurin](https://app.codecov.io/gh/apache/pinot/pull/14430/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <ø> (-61.76%)` | :arrow_down: | | [unittests](https://app.codecov.io/gh/apache/pinot/pull/14430/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | | | [unittests1](https://app.codecov.io/gh/apache/pinot/pull/14430/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | | | [unittests2](https://app.codecov.io/gh/apache/pinot/pull/14430/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | | Flags with carried for
Re: [PR] Add instance level consumer dir usage metric [pinot]
jasperjiaguo commented on code in PR #14430: URL: https://github.com/apache/pinot/pull/14430#discussion_r1838620339 ## pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManager.java: ## @@ -180,6 +181,24 @@ private void initInstanceDataDir(File instanceDataDir) { } } + @Override + public List getConsumerDirPaths() { +String consumerDirPath = _instanceDataManagerConfig.getConsumerDir(); + +if (consumerDirPath != null) { + return Collections.singletonList(new File(consumerDirPath)); +} else { + List consumerDirs = new ArrayList<>(); + String instanceDataDir = _instanceDataManagerConfig.getInstanceDataDir(); + for (String tableNameWithType : getAllTables()) { +String dirPath = instanceDataDir + File.separator + tableNameWithType + File.separator ++ RealtimeTableDataManager.CONSUMERS_DIR; +consumerDirs.add(new File(dirPath)); + } + return consumerDirs; Review Comment: should we store returnb value (consumerDirs) up so that we don't calculate these every time? We can add logging to it if it is calculated just once. ## pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManager.java: ## @@ -180,6 +181,24 @@ private void initInstanceDataDir(File instanceDataDir) { } } + @Override + public List getConsumerDirPaths() { +String consumerDirPath = _instanceDataManagerConfig.getConsumerDir(); + +if (consumerDirPath != null) { + return Collections.singletonList(new File(consumerDirPath)); +} else { Review Comment: this is when consuming and completed dir is mixed up right? could we add some comment to the code? -- 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
Re: [PR] Add instance level consumer dir usage metric [pinot]
sajjad-moradi commented on code in PR #14430: URL: https://github.com/apache/pinot/pull/14430#discussion_r1838620812 ## pinot-server/src/main/java/org/apache/pinot/server/starter/helix/BaseServerStarter.java: ## @@ -707,6 +711,23 @@ public void start() _serverQueriesDisabledTracker = new ServerQueriesDisabledTracker(_helixClusterName, _instanceId, _helixManager, serverMetrics); _serverQueriesDisabledTracker.start(); + +// Add metrics for consumer directory usage +serverMetrics.setOrUpdateGlobalGauge(ServerGauge.CONSUMER_DIR_USAGE, () -> { + List serverConsumerDirs = instanceDataManager.getConsumerDirPaths(); + final long[] totalSize = {0}; Review Comment: why array? Single long is not enough? ## pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManager.java: ## @@ -180,6 +181,24 @@ private void initInstanceDataDir(File instanceDataDir) { } } + @Override + public List getConsumerDirPaths() { +String consumerDirPath = _instanceDataManagerConfig.getConsumerDir(); + +if (consumerDirPath != null) { + return Collections.singletonList(new File(consumerDirPath)); +} else { + List consumerDirs = new ArrayList<>(); + String instanceDataDir = _instanceDataManagerConfig.getInstanceDataDir(); + for (String tableNameWithType : getAllTables()) { +String dirPath = instanceDataDir + File.separator + tableNameWithType + File.separator ++ RealtimeTableDataManager.CONSUMERS_DIR; +consumerDirs.add(new File(dirPath)); + } + return consumerDirs; Review Comment: My understanding is that if the consumerDir is not specified, the table is not mmapping the consuming segments (it uses direct offheap). So maybe we don't need the else part?! -- 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