transaction rewrite, increase pg connection timeout #1525
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
InoMurko
force-pushed
the
inomurko/1378-applyblock
branch
from
587bdf3
to
29a6099
Compare
|
@InoMurko Where you able to try this out with a 64k transactions block? Where would be the best place for us to introduce a test like this to ensure we don't break it in the future? |
|
My testing locally was with 50k transactions. Let me just reiterate that this PR doesn't introduce any performance increases. |
|
@InoMurko understood, but you did introduce a bigger timeout, which appears to solve the issue with 50k transactions, right? That's good, but not the maximum we can have in a block so we probably need to test with a bit more 🙇 . And I still would like to see those changes documented in a test ran somewhere in the pipeline. cc @kevsul |
|
Explaining the test in SQL terms: Transactions and outputs that "came out of that block". These are being chunked into equal parts. Where a single part has at most 0xFFFF items: Transactions:
same stuff, but for txoutputs
the last thing is to mark all existing inputs as spent, in this test case, its 64k updates, but the table is empty:and so on and so on... |
|
A limited amount of transactions in a block: |
|
Now I believe that the test example is completely false, I think its creating inputs out of thin air. A full block could be a perf test. cc @achiurizo @T-Dnzt |
|
The Postgres version we're using in GCS is 9.6. All three primary keys are being search for in the UPDATE. Could it be that extra references are slowing down the updates? |
achiurizo
approved these changes
May 28, 2020
| ] | ||
| ) | ||
| db_txs_stream = Chunk.chunk(db_txs) | ||
| db_outputs_stream = Chunk.chunk(db_outputs) |
There was a problem hiding this comment.
Lots of Chunk.chunk(chunk)
|
|
||
| result | ||
|
|
||
| {:error, changeset} -> | ||
| _ = Logger.debug("Block \##{block_number} not persisted in WatcherDB, done in #{insert_duration / 1000}ms") | ||
| {:error, name, changeset, explain} -> |
There was a problem hiding this comment.
{:error, name, changeset, explain}
T-Dnzt
approved these changes
May 28, 2020
mederic-p
approved these changes
May 29, 2020
added 10 commits
May 29, 2020 10:32
InoMurko
force-pushed
the
inomurko/1378-applyblock
branch
from
d2caa3e
to
0de38b3
Compare
added 2 commits
May 29, 2020 10:38
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#1378 (comment)
Overview
Rewritten the transaction into ectomulti.
Changes
Describe your changes and implementation choices. More details make PRs easier to review.
Testing
mix test
Table description: