-
Notifications
You must be signed in to change notification settings - Fork 484
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
Scan API and get_account_transfers #1054
Merged
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
batiati
commented
Jul 22, 2023
batiati
commented
Jul 22, 2023
batiati
force-pushed
the
batiati-scan-api
branch
from
October 18, 2023 14:12
9d2a2f5
to
412506a
Compare
batiati
force-pushed
the
batiati-scan-api
branch
2 times, most recently
from
November 3, 2023 13:13
9a44f9a
to
14abe83
Compare
batiati
force-pushed
the
batiati-scan-api
branch
from
November 22, 2023 16:02
df5a602
to
73031ed
Compare
batiati
force-pushed
the
batiati-scan-api
branch
from
November 28, 2023 18:02
fbae537
to
39cf3dd
Compare
batiati
force-pushed
the
batiati-scan-api
branch
from
December 13, 2023 11:50
8793e69
to
d5c3645
Compare
batiati
force-pushed
the
batiati-scan-api
branch
from
December 18, 2023 18:29
a7accdc
to
c385bdb
Compare
sentientwaffle
approved these changes
Dec 19, 2023
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.
LGTM! 🎉
sentientwaffle
added a commit
that referenced
this pull request
Dec 19, 2023
As part of the [Scan API](#1054) code review, we found that `Groove.prefetch()` would invoke its callback synchronously if all of the objects could be prefetched from cache. That was fixed as part of the same PR. But fixing it caused an unrelated test to fail: `replica_test.zig`'s `"Cluster: repair: ack committed prepare"`. During the `Change views. B1/B2 participate. Don't allow B2 to repair op=21.` step, even though only B1/B2 participated in the view change, A0 was still allowed to send SVC messages. Deferring `prefetch()`'s callback to next tick made commits take slightly longer. This permutation meant that B1/B2 would finish their view change (as before), but then A0 would prod them into kicking off another view change.
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.
This PR
Scan API, including fuzz tests and the new state machine operation
get_account_transfers
for querying transfers for a given account.Scan API
Scan API:
ScanTreeType
: The basic building block of the range queries, specialized overTree
, allowing to seek forkey_min
..key_max
in ascending or descending key order in any LSM tree.ScanBuilderType
: Specialized type overGroove
, groupingScanTreeType
s for allIndexTrees
in the groove, exposing them in a common API not specialized byTree
(theScan
Interface), allowing conditions where the predicate is the tree'sCompositeKey
prefix and the value produced is theObjectTree
's timestamp.Merge operations (union, intersection, difference) can be performed with scans that are agnostic about its source (e.g. it can be a
ScanTreeType
specialized over anyTree
or it can be another merge operation). TheScan
type holds all possible concrete implementations as a tagged Zigunion
for dynamic dispatch through comptime-generated code.ScanLookupType
: Implements the lookup logic for retrieving the groove objects from a scan. Unlike the regular prefetch mechanism, this lookup acts only overtimestamp
and loads the objects directly into a provided buffer, keeping the same sort order of the underlying scan. The buffer size can be used as theLIMIT
operator.This diagram illustrates how multiple scans can be combined to create a complex query:
API limitations
This API is intended to be used internally by the state machine, not to be exposed directly to the end user, it has some sensible limitations:
1. Only secondary indexes contained in
IndexTree
can be queried byScanGrooveType
.Examples:
✔ Supported:
✖ Not supported:
2. The field
timestamp
can be used in the query for paging, not as an index.Examples:
✔ Supported:
✖ Not supported:
3. Only objects of the same
Groove
can be queried together.✖ Not supported:
4. Merge operations on sorted scans.
Merge operations such as union, intersection, and difference can only be performed in scans sorted in the same direction.
Since scans always yield values sorted by
prefix, timestamp
, it's not possible to combine scans when the criteria aren't an exact match to theprefix
.Examples:
✔ Supported: Results sorted by timestamp:
✔ Supported: Results sorted by code and then timestamp:
✖ Not supported:
New
get_account_transfers
operationNew
get_account_code
operation:A new operation was implemented on top of the
StateMachine
using the Scan API to retrieve transfers related to anaccount_id
.Query
Transfers
bydebit_account_id
,credit_account_id
, or both; sorted by timestamp in ascending or descending order; optionally from a specific timestamp (exclusive range>
or<
depending on the sort order) and limited by a number of transfers.The combination of
timestamp
andlimit
can be used as pagination, passing the last timestamp received from the previous "page".NOTE: The operation
get_account_transfers
differs from the other operations by the fact it always accepts only oneEvent
and can return unknown amounts ofResult
s (limited by the message size).In many places in our clients, we assumed that the maximum number of
Results
could be inferred by the number ofEvent
s provided, making it necessary to expose the maximum size to the client side. Those places are signalized in the code with aTODO
label.Includes docs section for
get_account_transfers
and client support with autogenerated bindings, docs, samples, and tests.TODO:
MacOS is segfaulting in Debug mode. I could track the cause down the call of
Groove.init(...)
, but this function itself is never executed.EDIT It was caused by stack overflow declaring a
BoundedArray
.Unit and fuzz tests for
Scan
andMerge
will be included during the query engine implementation.get_account_transfers
unit tests need changes on the test executor. We are deferring the refactor to be done along the Query API.