[NC-2207] Integrate rocksdb-based queue into WorldStateDownloader #746
Conversation
| File queueDataFile = queueDataDirectory.toFile(); | ||
| queueDataFile.mkdirs(); | ||
| // Clean up this data for now (until fast sync resume functionality is in place) | ||
| queueDataFile.deleteOnExit(); |
There was a problem hiding this comment.
I don't think deleteOnExit will work for a directory that we'll wind up creating files inside. The naming of queueDataDirectory and queueDataFile is a bit misleading here as well since they're both actually a directory just of type Path or File. Not quite sure what the right naming is but I think I'd have queueDataDirectory and queueDataDir since they're the same thing in slightly different forms.
There was a problem hiding this comment.
Yeah, you're right - this won't clean up non-empty directories. I'm still testing this locally, so I'll clean it up a bit and ping you when its ready to look at.
There was a problem hiding this comment.
Added a shutdown hook for the cleanup: bf2306f
Add some logging to WorldStateDownloader. Add some methods to sync config builder.
| LOG.error("Unable to clean up fast sync files: {}", stateQueueDirectory, e); | ||
| } | ||
| })); | ||
| } |
There was a problem hiding this comment.
It would probably be better to put this in a stop() method which is called from Runner.close. Pantheon already sets up a shutdown hook to call Runner.close and then when tests shutdown Runner without killing the whole JVM this clean up will run then and not be delayed.
There was a problem hiding this comment.
Also, I can't see where the Queue gets closed. It didn't matter with the in memory implementation but we do need to ensure that rocksdb is closed. This may be a good place (though we'd need to ensure WorldStateDownloader is stopped and not going to try to access the queue anymore.
There was a problem hiding this comment.
Good feedback! Pushed the cleanup into a stop() method here: f3d4e81
There was a problem hiding this comment.
Yep, that all LGTM. Merge away. :)
Move fast sync cleanup to stop method
PR description
Integrate rocksdb-based queue into WorldStateDownloader.