tkalkirill commented on code in PR #2237:
URL: https://github.com/apache/ignite-3/pull/2237#discussion_r1243214953
##########
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/server/persistence/RocksDbKeyValueStorage.java:
##########
@@ -170,6 +172,12 @@ public class RocksDbKeyValueStorage implements
KeyValueStorage {
/** Snapshot manager. */
private volatile RocksSnapshotManager snapshotManager;
+ /**
+ * Revision listener for recovery only. Notifies {@link
MetaStorageManagerImpl} of revision update.
+ * Guarded by {@link #rwLock}.
+ */
+ private LongConsumer revisionListener;
Review Comment:
I think that the name is not quite suitable, it is better to indicate that
this is a listener on recovery, plus it would be better to use the new
interface instead of `LongConsumer`,
##########
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/server/persistence/RocksDbKeyValueStorage.java:
##########
@@ -311,6 +318,12 @@ protected void createDb() throws RocksDBException {
}
}
+ private void notifyRevisionUpdate() {
Review Comment:
You need to specify that the notification will be at the end of the recovery.
##########
modules/runner/src/main/java/org/apache/ignite/internal/app/IgniteImpl.java:
##########
@@ -706,14 +706,23 @@ public CompletableFuture<Ignite> start(Path configPath) {
LOG.info("Components started, joining the cluster");
return cmgMgr.joinFuture()
- // using the default executor to avoid blocking the CMG
Manager threads
+ .thenComposeAsync(unused -> {
+ LOG.info("Join complete, starting MetaStorage");
+
+ try {
+ lifecycleManager.startComponent(metaStorageMgr);
+ } catch (NodeStoppingException e) {
+ throw new CompletionException(e);
+ }
+
+ return metaStorageMgr.recoveryFinishedFuture();
Review Comment:
Since the recovery will end here, maybe it makes sense to get rid of
`RecoveryCompletionFutureFactory` or create a ticket for this?
##########
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/server/KeyValueStorage.java:
##########
@@ -292,4 +294,11 @@ public interface KeyValueStorage extends ManuallyCloseable
{
* @return Timestamp corresponding to the revision.
*/
HybridTimestamp timestampByRevision(long revision);
+
+ /**
+ * Sets the revision listener. This is needed only for the recovery, after
that listener must be set to {@code null}.
+ *
+ * @param listener Revision listener.
+ */
+ void setRevisionListener(@Nullable LongConsumer listener);
Review Comment:
I think that the name is not quite suitable, it is better to indicate that
this is a listener on recovery, plus it would be better to use the new
interface instead of `LongConsumer`,
What is the meaning of **null** listener?
--
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]
