add support for ancient storage as json-rpc service #168
Conversation
add support for ancient storage as json-rpc service This change adds support for decoupling ancient storage from disk to a json-rpc supported service. It defines FreezerRemoteClient as the interface that services must implement in order to be an ancient storage provider. This change will allow developers to build custom long term storage solutions for ancient blocks. The protocols supported for a storage provider are as follows, http,https,ws,wss and ipc. The remote ancient storage can be accessed via the newly added ancient.rpc flag provided at the commandline. This may not be used in conjunction with ancient.datadir
|
this PR is the squashed version #103 |
meowsbits
changed the title
meowsbits
force-pushed
the
feat/ancient-remote
branch
from
8509839
to
7c052d0
Compare
…ization See comment for detailed reasoning. Installs validation of kv/ancient stores when using remote freezer. Signed-off-by: meows <b5c6@protonmail.com>
…eezer Signed-off-by: meows <b5c6@protonmail.com>
core/rawdb/freezer.go
Outdated
| // Retrieve the freezing threshold. | ||
| hash := ReadHeadBlockHash(nfdb) | ||
| if hash == (common.Hash{}) { | ||
| log.Debug("Current full block hash unavailable") // new chain, empty database | ||
| backoff = true | ||
| continue | ||
| } | ||
|
|
||
| numFrozen, err := f.Ancients() |
There was a problem hiding this comment.
Just a call for 👀 .
Using an assigned value like this rather than having direct access to the 'live' f.frozen field is a drastic logical step. We need to be sure that the value will always be updated in sync with the subsequent logic depending on it, and that we're never going to be thrashing the RPC in this loop.
There was a problem hiding this comment.
And this modifies the built-in FS ancient database mechanism. So, double double check.
There was a problem hiding this comment.
An alternative here that may actually be preferable would be to duplicate the logic into
// original, unmodified FS implementation, using f.frozen and unmodified function signature
func (f *freezer) freeze(db ethdb.KeyValueStore) {
// freezer client implementation, using f.Ancients() and modified function signature
func freezeRemote(db ethdb.KeyValueStore, f ethdb.AncientStore, quitChan chan struct{}) {NewDatabaseWithFreezer and NewDatabaseWithFreezerRemote are the only two callers, so the refactoring and code dependencies are simple.
There was a problem hiding this comment.
I agree with that then we can reduce the surface area of things that can go wrong and prove out that ancient remote works as expected over a longer time frame.
There was a problem hiding this comment.
f528c61 implements this proposal ☝️. Can easily revert if we decide against.
There was a problem hiding this comment.
yeah I think this makes the most sense for now, it gives us some space to prove out the remote freezer logic. any thoughts on keeping track of any critical upstream patches to freezer within remote freezer?
There was a problem hiding this comment.
I mean, to me the code before f528c61 looked OK too (with f.frozen using f.Ancients() instead). The only reason for the refactoring to near-dupe functions is to reduce the risk in case I'm wrong. As @zcstarr pointed out however, duplication introduces an overhead in code-life audits and maintenance.
There was a problem hiding this comment.
for the record here, I'd say the code looks correct for f.Ancients() as well.
Signed-off-by: meows <b5c6@protonmail.com>
Signed-off-by: meows <b5c6@protonmail.com>
Since the sibling FS freezer implementation doesn't use a mutex around its methods, I think we can safely remove this. If freezer implementations want to implement a mutex at the server level, they can. Call atomicity should be managed by the caller and the implementer, not the middleman. Signed-off-by: meows <b5c6@protonmail.com>
Signed-off-by: meows <b5c6@protonmail.com>
…impl The 'freeze' method was, in the course of this change set, refactored to permit use by both the remote freezer and the local freezer implementations. The risk with this approach is that an unforeseen bug would cause the apparently-minor necessary modifications to the logic to negatively impact functioning of the default builtin FS ancient store implementation. This refactoring near-duplicates the logic so that the builtin ancient store freezer.freeze method is now unmodified and unimpacted by the remote freezer client, which gets its own dedication goroutine function now. This reduces the 'risky' surface area of code modification against the existing code, albeit at the cost of a function's non-reuse. Signed-off-by: meows <b5c6@protonmail.com>
|
I just tested:
|
Conflicts:
eth/backend.go
node/service.go
Signed-off-by: meows <b5c6@protonmail.com>
|
final merged changes looked good |
add support for ancient storage as json-rpc service
This change adds support for decoupling ancient storage from
disk to a json-rpc supported service. It defines FreezerRemoteClient
as the interface that services must implement in order to be an
ancient storage provider.
This change will allow developers to build custom long term storage
solutions for ancient blocks.
The protocols supported for a storage provider are as follows,
http,https,ws,wss and ipc.
The remote ancient storage can be accessed via the newly added
ancient.rpc flag provided at the commandline. This may not be used
in conjunction with ancient.datadir