Nikita-Shupletsov commented on code in PR #20749:
URL: https://github.com/apache/kafka/pull/20749#discussion_r2453532998


##########
streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBStore.java:
##########
@@ -157,9 +159,14 @@ public RocksDBStore(final String name,
     @Override
     public void init(final StateStoreContext stateStoreContext,
                      final StateStore root) {
+        initialized.set(true);
         // open the DB dir
         metricsRecorder.init(metricsImpl(stateStoreContext), 
stateStoreContext.taskId());
-        openDB(stateStoreContext.appConfigs(), stateStoreContext.stateDir());
+        if (!open) {
+            preInit(stateStoreContext);

Review Comment:
   maybe it's not a valid concern, but it looks like this interface would be 
hard to explain and thus use correctly: there is a two stage init process. 
sometimes we call preInit(phase one) explicitly, but there are a lot of cases 
when the author of the store needs to call it themselves in the init(phase two).
   
   as we rely on stateStore.isOpen be true, maybe we should communicate that 
and follow the same path every time we want to initialize a store?
   i.e.
   * phase one(preInit/open)
   * check if opened
   * phase two(init)
   
   it will slightly complicate the implementation on our side, but will make it 
significantly easier to follow for users



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