-
Notifications
You must be signed in to change notification settings - Fork 473
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: fix RpcDB connection so one can attach local Instance to remote DB for debugging purpose #8049
base: master
Are you sure you want to change the base?
Conversation
Co-authored-by: Lukasz Rozmej <[email protected]>
@@ -96,7 +97,7 @@ private byte[] GetThroughRpc(ReadOnlySpan<byte> key) | |||
byte[] value = null; | |||
if (response.Result is not null) | |||
{ | |||
value = Bytes.FromHexString((string)response.Result); | |||
value = Bytes.FromHexString(response.Result.ToString()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cosmetic I prefer $"{response.Result}" for strings
@@ -59,7 +59,7 @@ private DbSettings GetRocksDbSettings(DbSettings originalSetting) | |||
bool firstDb = _index == -1; | |||
|
|||
// if first DB, then we will put it into main directory and not use indexed subdirectory | |||
string dbName = firstDb ? originalSetting.DbName : originalSetting.DbName + _index; | |||
string dbName = originalSetting.DbName; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmmm, my intuition is telling me that it could cause issues with full-pruning
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah wrote to Ashraf already as was not confident about this one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But path still point to proper location - just DbName is different - without this change RPC was looking for a DB called "State0"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be fine 'Path' is more important and 'Name' is mostly for our convenience. You might want to change FullPruningInnerDbFactoryTests.MatchSettings
Fixes RpcDB mode - with proper configuration it allows to debug remote DB (or at least use some remote data and debug RPC calls issues).
To make it work, make sure local node runs with flags:
--Init.DiagnosticMode=RpcDb --Init.RpcDbUrl=JsonRPC URL (properly formatted with http:// as prefix)
Also enabling of debug rpc endpoints is needed as in background few debug calls are made to fetch from remote DB
Also for now it is quite slow (at least on my end) so worth applying --Init.ProcessingEnabled=false and disabling CL for processing on the desired node so will avoid messages about potential corruption of Db and will easily proceed to a enabled rpc node
Types of changes
What types of changes does your code introduce?
Testing
Requires testing
If yes, did you write tests?