dxichen commented on code in PR #1697:
URL: https://github.com/apache/samza/pull/1697#discussion_r1543808370


##########
samza-core/src/main/scala/org/apache/samza/storage/ContainerStorageManager.java:
##########
@@ -192,6 +193,21 @@ public ContainerStorageManager(
     this.storeConsumers = 
ContainerStorageManagerUtil.createStoreChangelogConsumers(
         activeTaskChangelogSystemStreams, systemFactories, 
samzaContainerMetrics.registry(), config);
 
+    // The store directory paths are used by SamzaContainer to add a metric to 
watch the disk space usage of the store
+    // directories. The stores itself does not need to be created but the 
store directory paths need to be set to be
+    // able to monitor them, once they're created and in use.
+    Set<String> storesToCreate =
+        
ContainerStorageManagerUtil.getNonSideInputNonInMemoryStores(storageEngineFactories,
 sideInputStoreNames, config);
+    for (String storeName : storesToCreate) {
+      for (Map.Entry<TaskName, TaskModel> task : 
containerModel.getTasks().entrySet()) {
+        File storeDirPath =
+            ContainerStorageManagerUtil.getStoreDirPath(storeName, config, 
activeTaskChangelogSystemStreams,
+                sideInputStoreNames, task.getKey(), task.getValue(), 
storageManagerUtil, loggedStoreBaseDirectory,
+                nonLoggedStoreBaseDirectory);
+        storeDirectoryPaths.add(storeDirPath.toPath());

Review Comment:
   As discussed offline, lets refactor 
`ContainerStorageManagerUtil.createInMemoryStores` and similar to accept the 
`storeDirectoryPaths` instead of  relying on pass-by-reference to mutate `this. 
storeDirectoryPaths` in order to clarify that this field is updated. 
Furthermore, instead of keeping this path creation in the ctor I would prefer a 
static helper function like `getStoreDirPathsForStoreNames` run or each types 
of stores



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to