bkonold commented on a change in pull request #1343:
URL: https://github.com/apache/samza/pull/1343#discussion_r417013068



##########
File path: 
samza-core/src/main/scala/org/apache/samza/storage/ContainerStorageManager.java
##########
@@ -110,13 +114,13 @@
 public class ContainerStorageManager {

Review comment:
       In terms of conceptual responsibility, not much about CSM is changing in 
this PR. It is still responsible for lifecycle of side input consumption 
(refactoring side input processing logic out of this class is already a small 
win), and for one background thread necessary for side input coordination.
   
   I'd taken a stab at a larger refactor, and I think it would make a lot of 
side to remove the handling of side inputs from this class entirely. There is 
not a lot of dependency between storage management of side input and non-side 
input stores. This would split container storage management into two classes, 
with their lifecycles each managed at the SamzaContainer level.
   
   I tried not to do that in this PR since it is already huge. Let's discuss 
further and decide what we want to do in this PR and want we want to document 
for future refactor effort.




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


Reply via email to