-
Notifications
You must be signed in to change notification settings - Fork 152
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
replace freezer with a custom implementation #103
Conversation
This is just a dummy PR just so we can quickly look at the code and diff it as we work out what's the best approach to adding a more generic and flexible thing to "servicize or decouple" the freezer |
f851ea1
to
c6a9968
Compare
@zcstarr 🚧 Just rebased on master, |
Just pushed changes which sketch adding support for AWS S3 as the remote freezer. You'll need to have your AWS SDK credentials set (https://docs.aws.amazon.com/sdk-for-go/v1/developer-guide/configuring-sdk.html). In the script below I have creds as:
Note the CLI flag change, now with env AWS_PROFILE=developer-s3 AWS_REGION=us-east-1 ./build/bin/geth \
--classic \
--ipcpath=~/.ethereum/geth.ipc \
--datadir ~/SAMSUNG_T5/e/sandbox/remote-freezer-develop \
--verbosity 4 \
--datadir.ancient.remote=s3 \
--datadir.ancient.remote.namespace=coregeth-classic-cataclysm |& tee geth.log NOTE: S3 demands that bucket names are globally unique. So you'll need to change
Eventually caching will be critical for performance, since |
To nuke your buckets between development versions: > cat ./geth-s3-buckets-nuke.sh
#!/usr/bin/env bash
env AWS_PROFILE=developer-s3 AWS_REGION=us-east-1 aws s3 rb s3://<my-bucket> --force
|
A few thoughts. Features for S3 as-is:
Extensibility-oriented:
|
So, as-is with the current schema, which adheres strictly to the freezerTable schema (hashes, headers, bodies...) and uses block number as the index, I'm seeing about 2 blocks/second upload, which will take about 2 months to upload for the classic chain's current height. Further, creating 50million relatively tiny objects may have negative implications for the S3 request costs, where I expect fewer, larger objects will be cheaper. IMO this is beyond usable for most cases, so we need to find a way to make uploads more efficient. My understanding so far is that most of our upload time is due to transaction overhead (even though we're Grouping resources by block (instead of by kind/block) saw about 8 blocks/second upload. I'm going to try reverting to this pattern, but then extending it further to use 32-block objects, with each containing all the tables. If this moves the upload time from 2 blocks/second to There is probably a further way of managing this by using an AWS Lambda to partition or group the objects once they're uploaded, but I'm going to call that out-of-scope for now. As-is: |
Just thinking out loud here. I'm coming back to 1 block objects. This keeps GETs streamlined. Will cost about 50USD at the without-any-batching ceiling to set up (upload entire ancient store), mostly in transaction counts. Batching requests in 2048-block chunks (normal during sync) will bring the number down to 10M / 2048 =~ 5k, costing just a few cents in PUT requests. The downside still is time, where I'm estimating 2-3 weeks for an upload-enabled sync. Although I'd consider this on the upper end of desirable, I'm currently of the opinion that it is tolerable. Once a user has a canonical bucket uploaded, they can use |
This is awesome,
I guess what's behind the slow block upload speed, is it because it seeks on s3 prior to upload? Or is it simply not using a persistent connection to upload? |
To be honest I'm not exactly sure. My googling and esoteric forum lurking lead me to "request transaction overhead"-type explanations. Batching (the new SDK API) helps a lot, I'm sure, and although we're typically (during sync) rolling 2048 blocks into a batch, I'm not sure that we're actually making the most of that optimization opportunity. As-is, we are maintaining a single There may also be "transaction overhead" beyond the simple HTTP requests, ie something on the Amazon end with what it costs them (transaction-wise) to create objects -- and we're creating a significant number of rather small objects. They charge per-PUT, so I have to think that although it's in USD "cheap"(-ish), it may speak to the apparent slowness. In general my intuition is that S3 for millions of tiny things is probably less ideal than S3 for hundreds of thousands of medium-sized things, etc. With that said, still, I am not completely satisfied with it as an explanation for why 10M small objects take 2-3 weeks to upload to S3 (although one will find horror stories of long waits to delete large numbers of objects from buckets in the forums). Further investigation might want to create an isolated test case that's totally separate from this blockchain implementation and just try fiddling around with configurations and implementation to get a feel if it's our implementation or if it's just gonna be something like this no matter what. |
last 2 commits fix:
|
Worth noting that AWS S3 Region chosen will also matter a lot, as well as internet connection (for example, AWS seems to not like some of my VPN endpoints). |
With latest push I'm getting around 150-200 blocks/second. Latest push now uses 32-block groups for object storage. For example, object And I turned my VPN off, so my internet is now on the same continent as my AWS bucket region. And initialization from S3 freezer seems to work (nuke your local db between runs):
|
This happens on restart quite often:
|
#103 (comment) Signed-off-by: meows <b5c6@protonmail.com>
Adding an a comment here. latest commit supports ancient remote as a json-rpc service
then
The commands on both side need a little tlc, the client side should just take an rpcPort to connect to and a host, the @meowsbits let's sync soon and talk about restructuring the internals. |
…mplementation that's toggled by flag
'freeze' function was implemented as a method of freezer instances, where freezers should be objects that satisfy the AncientStore interface defined in ethdb/database.go Implementing the experimental freezerRemote object adjacent to freezer object demonstrated the brittle way the actual freezing logic is implented, where that logic is responsible for transitioning data from the KV leveldb instance to the freezer. This change allows the 'freeze' logic to be shared between the (now) two freezer implementations. Signed-off-by: meows <b5c6@protonmail.com>
Variable declarations default to the type's zero value, so assigning to nil is ineffectual. Signed-off-by: meows <b5c6@protonmail.com>
These log lines cause console tests testing UI output to fail, ala go test ./cmd/... Commenting them will keep our CI useful, while leaving breadcrumbs that are toggleable and searchable. Signed-off-by: meows <b5c6@protonmail.com>
This must have been erroneously added somewhere along the line. AFAIK it is completely unrelated to anything. Signed-off-by: meows <b5c6@protonmail.com>
numFrozen replaces f.frozen (since the iterface doesn't provide a field), replacing it with a function call to frdb.Ancients, which provides the same value. However, since we're not accessing the field directly, we need to be careful about how we handle the logic around the assigned value. The most implementation most equivalent to prior behavior would be be call the method every time in lieu of f.frozen. But this would be a lot of calls, and probably quite a bit slower. The value of numFrozen will increment +1 every time AppendAncient succeeds, so that logic is added. And rather than assigning the value once prior to the for loop, the value is assigned with every iteration of the loop via the method Ancients() call. backoff logic is expected to handle method thrashing in the same way that it wants to handle avoiding db thrashing. Signed-off-by: meows <b5c6@protonmail.com>
…oint -> --ancient.rpc - Remove IPC vs. HTTP/WS transport segregation, along with node.Config.__IPC bool. We originally segregated IPC thinking it would reduce ambiguity in configuration, but at this point I think it causes an unworthy amount of finnicky and adhoc logic through the configuration and constructor codes. This change uses the exiting rpc.Client.Dial implementation as the reference implementation, and simplifies the remote ancient flags to one: --ancient.rpc. On the backend, then, this flag value is simply passed to that rpc method. The node package validates the URL with copy-pasted code from the RPC package when necessary (for handling an empty-value default... which arguably it shouldn't be doing at all anyways.) Signed-off-by: meows <b5c6@protonmail.com>
Signed-off-by: meows <b5c6@protonmail.com>
Signed-off-by: meows <b5c6@protonmail.com>
Prevents false-negatives if test environment doesnt have accessible networking, or if a preexiting colliding geth service, eg. port 30303 in use. Signed-off-by: meows <b5c6@protonmail.com>
fi will be nil if DNE Signed-off-by: meows <b5c6@protonmail.com>
…endpoint - Install configurable env var GETH_ANCIENT_RPC to allow user to define an external endpoint handled by an independent freezer server instance. This can be a socket, http, ws, or stdio. Note that since the AncientStore API doesn't have a reset-button, running multiple tests against the same freezer instance may have negative outcomes, since the data stores will not represent consistent collection (chains). If no external freezer is configured, the default will create an ephemeral memory-packed freezer server and connect to it over a temporary IPC socket. Signed-off-by: meows <b5c6@protonmail.com>
This is a kind of manual reset for each test which should allow a single remote freezer server instance to handle multiple tests run serially. The close method will be called likewise, so freezer servers sensitive to that may be negatively effected. Signed-off-by: meows <b5c6@protonmail.com>
Just updated #103 (comment) with latest script, including latest changes to flag design, ie. 76f8b65 adds a configurable Here's my script for that, using the included memory-backed ancient store command #!/usr/bin/env bash
set -x
make all
echo "Starting ancient-store-mem (ephemeral remote ancient store)..."
(
./build/bin/ancient-store-mem ./ancient.ipc > ancient-store.log 2>&1
)&
aspid=$!
trap "kill -9 $aspid"
echo "Running tests..."
[ ! -S ./ancient.ipc ] && echo "Must have running freezer server with ipc at ancient.ipc" && exit 1
env GETH_ANCIENT_RPC="$(pwd)/ancient.ipc" go test -count=1 -v -run='_RemoteFreezer' ./core/... &> test.out
exited=$?
tail -40 test.out
echo "Finding unique method names called (to show completeness in method exercise)"
cat ancient-store.log | grep 'mock server called' | cut -d'=' -f2 | cut -d' ' -f1 | sort | uniq
[ $exited = 0 ] && echo "PASS" || echo "FAIL"
exit $exited |
Signed-off-by: meows <b5c6@protonmail.com>
Signed-off-by: meows <b5c6@protonmail.com>
Signed-off-by: meows <b5c6@protonmail.com>
Signed-off-by: meows <b5c6@protonmail.com>
If testing against a non-ephemeral ancient db, a non-zeroed db would fatally contaminate tests. Signed-off-by: meows <b5c6@protonmail.com>
New command uses pre-existing builting FS ancient store wrapped in an RPC server. It doesnt work yet. And the export-refactoring touched a lot of comments that it probably shouldn't have. Probably revert me. Signed-off-by: meows <b5c6@protonmail.com>
Signed-off-by: meows <b5c6@protonmail.com>
This logic was checking and mutating a freezer url in a way that was a vestige of how the freezer path is handled for the builtin freezer fs implementation. Instead of using supervisory logic, we can just let the raw value pass through to the rpc package where it will be validated and used in context. Any errors coming from that context will be raised. This change also includes a tidbit of a wider parameter renaming that I'm going to commit next. Signed-off-by: meows <b5c6@protonmail.com>
This gives descriptiveness and consistency. Signed-off-by: meows <b5c6@protonmail.com>
Signed-off-by: meows <b5c6@protonmail.com>
Signed-off-by: meows <b5c6@protonmail.com>
This was a sketch of a good idea, but is out of scope for the current feature. The deleted code established a remote freezer server that would behave exactly like the built in filesystem freezer, but do so via RPC communication, and thus be able to replicate geth's ancient datadir store remotely and possibly for multiple clients. But not yet. Signed-off-by: meows <b5c6@protonmail.com>
This reverts commit 1ab7cab. This change made a big programmatic refactoring which exported the rawdb.freezer struct and constructor. This was intended to enable the hypothetical cmd/ancient-store-fs command, which has since been abandoned for now.
Signed-off-by: meows <b5c6@protonmail.com>
Apparently it takes a lil longer for indices initialization here. Or I just got unlucky. Signed-off-by: meows <b5c6@protonmail.com>
Closed via the merge of #168 |
…mplementation that's toggled by flag