rpuch commented on a change in pull request #723:
URL: https://github.com/apache/ignite-3/pull/723#discussion_r829886895
##########
File path:
modules/metastorage-server/src/main/java/org/apache/ignite/internal/metastorage/server/persistence/RocksDbKeyValueStorage.java
##########
@@ -218,78 +223,33 @@ private void destroyRocksDb() throws RocksDBException {
public void close() throws Exception {
IgniteUtils.shutdownAndAwaitTermination(snapshotExecutor, 10,
TimeUnit.SECONDS);
Review comment:
So, in our case, this would mean that `MetaStorageManager` (the class
that now stops `RocksDbKeyValueStorage`) would need to make sure that all the
futures are resolved? Currently, its `stop()` method does not seem to bother
about it explicitly, but maybe I'm missing something here.
If we push this responsibility to the enclosing component, then this must be
a part of the contract, so the contract grows a bit more, and it becomes a
little bit less explicit (as the job to deal with such hanging futures is not
encapsulated close to the executor). I'm not sure if this is bad or not, to be
honest.
Also, we still need this to be done at the enclosing component side, and it
does not seem that it's done there yet (again, maybe I just don't see it). Not
in this PR, of course.
--
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]
