Batch reprepare speculatively #531
Conversation
As reprepare logic was duplicated (in prepared statements and in batches), I've extracted it as a separate function.
|
I've noticed I perform all prepares sequentially, effectively contradicting the purpose of this PR. Will change it into concurrent reprepare. |
wprzytula
force-pushed
the
batch-reprepare-speculatively
branch
from
6597b1c
to
0041016
Compare
|
v2: made reprepares concurrent. |
Before, each unprepared statement in a batch caused a full roundtrip to Scylla. We instead speculatively prepare them all, as there is strong likelihood that once one of them becomes unprepared, the others do too. This change may significantly reduce latency in such case.
|
On the other hand, this is pessimizing for situations where the number of invalidated statements is orders of magnitude smaller than the number of statements in the batch. We currently implement a variant of LRU with two pools (ref: scylladb/scylladb@1a9c6d9fd3) and this speculative preparation may interact badly with the algorithm by pushing other prepared statements from the first, probationary buffer, even though lots of them might have already been prepared. So, in order to judge how we should proceed with this, here's question 1: was this pull request based on an existing issue that somebody (or our test case) discovered, or was it a result of code inspection? If it's not based on a real issue, we at least need a couple of tests to prove that it makes sense, comparing what happens when a minority and majority of batch statements are not prepared, respectively. |
|
(btw, the commit which deduplicates repreparing logic is good on its own anyway, so we can definitely apply that one regardless of the result of this discussion) |
| _ => None, | ||
| }) | ||
| .map(|prepared_statement| { |
There was a problem hiding this comment.
It's probable that a single batch will have multiple instances of the same prepared query, only with different arguments - for example imagine 100 inserts to the same table. It would be good to deduplicate them before repreparing.
There was a problem hiding this comment.
I assume you mean deduplicating by prepared_id, am I right?
It was based solely on code inspection. |
Extracted repreparing as a method on Connection to avoid duplication. Also, after one statement in a batch is found to be unprepared, we speculatively prepare all prepared statements in the batch, as there is likelihood that once one of them becomes unprepared, the others do too.
Fixes: #529
Pre-review checklist
Fixes:annotations to PR description.