Read mutations via low and high watermarks #1045
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1045 +/- ##
==========================================
- Coverage 66.25% 65.16% -1.09%
==========================================
Files 39 39
Lines 2670 2733 +63
==========================================
+ Hits 1769 1781 +12
- Misses 587 632 +45
- Partials 314 320 +6
Continue to review full report at Codecov.
|
gdbelvin
force-pushed
the
f/retry/queue
branch
from
45ab222
to
56aa32b
Compare
gdbelvin
force-pushed
the
f/retry/queue
branch
from
d5f24e0
to
97d3a81
Compare
gdbelvin
force-pushed
the
f/retry/queue
branch
from
97d3a81
to
22af5d6
Compare
core/sequencer/sequencer_api.proto
Outdated
| @@ -25,6 +25,12 @@ package google.keytransparency.sequencer; | |||
| import "google/protobuf/empty.proto"; | |||
| import "v1/keytransparency.proto"; | |||
|
|
|||
| message MapMetadata { | |||
| // high_watermark is the highest timestamp (inclusive) of the source log that | |||
There was a problem hiding this comment.
What units are the timestamp in? (Hopefully something strictly monotonic...)
Can there be multiple entries with identical timestamps? If so, presumably the high watermark encompasses all of them?
There was a problem hiding this comment.
The timestamps are in nanoseconds from the epoch (January 1, 1970 UTC).
Added tests in sql/queue_test.go for
- Timestamp uniqueness - enforced by
Timebeing part of the primary key - Monotonicity - enforced by querying for
MAX(Time)and inserting in a single transaction.
core/sequencer/server.go
Outdated
| } | ||
|
|
||
| // Read mutations | ||
| batch, err := s.queue.ReadQueue(ctx, in.DomainId, low.HighWatermark, high) |
There was a problem hiding this comment.
The CT sequencers (both old and new) have a guard window to avoid sequencing things that are super-recent; I'm not sure exactly what problem that was intended to solve, but probably worth investigating to seem if the same concern applies here.
Ensure that queue.Send fails with a retryable error code.
Improve MapMetadata by defining both low and high timestamps. Also reuse MapMetadata in the CreateEpoch request.
* Create a metadata proto to describe the low and high watermarks for a map revision. * Save this metadata proto in the SetLeaves API * Use this metadata proto to perform reads during CreateEpoch.
Rework PeriodicallyRunBatchForAllDomains into PeriodicallyRun. This makes the function useful for all kinds of uses. Move calling PeriodicallyRun from Env to the test cases themselves. Some tests want tight control over the sequencer, while other tests want it to be automatically running in the background.
Now that we've from accepting mutation directly in the CommitBatch API to using Metadata watermark references into the mutation queue, we need to actualy use the KT front end for queueing mutations in all tests.
gdbelvin
force-pushed
the
f/retry/queue
branch
from
2fb7429
to
d3c3246
Compare
|
@daviddrysdale, many thanks for your review. As a result, I've added a bunch of unit-tests and reworked the This had some follow on effects as a few integration tests were "cheating". Tests that wanted tight control over what went in a particular revision were sending their mutations directly to |
We are not reusing the prepared statements, and thus are not deriving any benefit from them.
* master: Return error code for old requests. (google#1052)
|
Addressed feedback so far (I think). PTAL |
* master: Cleanup tink errors (google#1055) Generate consistency proofs in test vectors (google#1053) Merge Tink API changes (google#1054)
(CurrentMapRoot.HighWatermark, CurrentHighWatermark]