Repository navigation
Client SDK, CLI and Python for secondary indexes (refs #180) - #270
Open
shrayarora8 wants to merge 11 commits into
Open
shrayarora8 wants to merge 11 commits into
shrayarora8 wants to merge 11 commits into
Conversation
Four gaps in the composite-key storage layer: - LevelDB GetRange returned index entries as if they were user data. GetAllItems and GetKeyRange already skipped them; GetRange was missed. - DuckDB's composite-key methods returned 0 (success) while doing nothing. They now return -1. - EncodeCompositeKeyPrefix with no attributes added an extra \0, so a query for a whole index matched nothing. - DecodeCompositeKey didn't check for the \0 after "ck", so "ckXidx..." decoded as a valid entry. Each fix has a test. Nothing changes for keys that don't start with "ck\0".
Adds CREATE_INDEX_ENTRY, DELETE_INDEX_ENTRY, UPDATE_INDEX_ENTRY and QUERY_BY_INDEX so clients can use the composite keys added in the storage layer. The client sends the parts (index name, attributes, primary key) and the executor builds the stored key, so a client can't write any key outside the "ck\0" range. SET and SET_WITH_VERSION now refuse keys in that range. - Create fails if the record doesn't exist. - Delete and update fail if the entry doesn't exist. - Query returns each matching primary key once, sorted. - ExecuteRequest and ExecuteData both call one shared helper. Tests: kv_executor_index_test runs 21 cases on MemoryDB and LevelDB, through both ExecuteData and ExecuteRequest.
Values are opaque bytes, so a record can hold an image or any other binary data. Adds a test that stores PNG-like bytes (including zero bytes), indexes the record and queries it back. Also documents the one limit: a record whose value is empty can't be indexed, because it looks the same as a missing record.
A query returns the primary keys of the matching records, so an application that wants the records has to fetch them one by one, and each of those is its own request through consensus. With with_values set, the executor reads the matching records itself and returns them in the same reply, so a query costs one request whatever the match count. Off by default, so a query still returns keys only unless it is asked for.
Four methods matching the executor commands: CreateIndexEntry, DeleteIndexEntry, UpdateIndexEntry and QueryByIndex. Callers pass the index name, the attribute values and the primary key; the server builds the stored key. Unlike Set, these wait for the servers' reply, so a rejected request is visible to the caller. They return 0 on success, -3 if the servers rejected the request, or the send error from SendRequest (-1, -2). -3 is new because this is the first KV command the server can refuse; the existing codes keep their meanings.
Adds create_index_entry, delete_index_entry, update_index_entry and query_by_index, with --index, --attrs, --new_attrs and --pk. Attributes are comma-separated, e.g. --attrs shray,jpeg. Leaving --attrs off a query returns every entry in the index. Rejected requests print "(rejected by the servers)" instead of a bare return code.
Adds create_index_entry, delete_index_entry, update_index_entry and
query_by_index to the pybind module and to kv_operation.py, wrapping the
KVClient methods. Attributes can be a list, or a plain string for a
single-attribute index:
kv.create_index_entry("by_city", "Davis", "user:1", config)
kv.query_by_index("by_city", "Davis", config) -> ["user:1", ...]
kv_operation.py also looks for the module under bazel-bin/api. It only
looked in bazel-out/k8-fastbuild, which exists on Linux but not on other
platforms.
Starts a local cluster with LevelDB and runs the index commands against it: create, query by one and by several attributes, query the whole index, the three rejected cases, update, delete, and a restart to check the index is on disk. Every write is confirmed with a read, because set does not wait for consensus. Prints PASS and exits 0, or names the first check that failed and exits non-zero, so CI can run it.
long_options[] had no {NULL, 0, NULL, 0} terminator, so getopt_long read
past the end of the array and any unrecognised long option crashed the
tool. Adding the four index flags made that easy to hit, because the
natural guesses (--index_name, --attributes, --primary_key) are all
unknown names. Verified before the fix: exit 139 with no message; after
it, the usage text.
The index commands now also set the process exit status (0 accepted,
1 rejected or failed), so scripts can check them.
End-to-end test:
- a query that never reached the servers printed nothing after its header
line, which looked exactly like "no matches" and made four checks pass
against a dead cluster. The header is now required before the output is
used.
- added a check that a range scan returns no index entries, which is what
the GetRange filter is for.
- refuse to run outside the repository root, and stop the cluster on exit
or Ctrl-C.
Also corrects the reason given in a pybind comment and widens the Python
docstrings, which described -3 more narrowly than it behaves.
Adds with_values to KVClient::QueryByIndex, a --with_values flag to the CLI, and query_records_by_index to the Python module, which returns (key, value) pairs. The server does the reading, so an application that wants the matching records gets them in one request instead of one request per match.
Measures four ways to answer "which records have city = Davis?" against a running cluster: scan every record and filter in the application, query the index for keys only, query then fetch each match, and query with --with_values. Takes a client config path, so the same binary measures a local cluster or a multi-machine deployment. --records, --selectivity and --value_size set the dataset; --threads loads it with several clients, because the proxy batches 100 requests or 100ms and a single client pays that wait on every request. The four approaches must agree on the match count or the run fails, which already caught two mistakes in the benchmark itself.
This branch has not been deployed
This file contains hidden or 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
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.
Depends on apache/incubator-resilientdb#269. The first four commits belong to that PR.
What this PR does
#269 added four commands to the
server:
CREATE_INDEX_ENTRY,DELETE_INDEX_ENTRY,UPDATE_INDEX_ENTRYandQUERY_BY_INDEX.This PR adds the three ways an application can call them through the C++ client
(
KVClient), the command line tool, and the Python module. It also adds an end-to-end test and abenchmark against a running cluster.
Today, to answer "which users live in Davis?",an app fetches every record and filters them in its own code, or copies everything into MongoDB and queries it there. After this PR the app can ask ResilientDB directly, in one call.
Which apps can use it after this PR, and which still can't. Anything that links to the C++ client
or uses the Python module can use it now. Apps that talk to ResilientDB over HTTP (ResLens, resdb-orm, the Rust SDK and ResVault) cannot yet. These apps go through
ecosystem/graphql/service/http_server/crow_service.cpp, which uses its own client,sdk::ResDBKVClient, and that class does not inherit fromKVClient. Giving it the four methods and adding one route is the next PR.Using it
Command line.
rdbbelow iskv_service_tools --config <client config>:Python.
One attribute can be a plain string, as above. For an index with several attributes, you can pass a list like,
kv.query_by_index("by_owner_format", ["shray", "jpeg"], config).C++.
An index with more than one attribute
Say a photo app stores photos and wants to find them by owner and by file format. It creates an
index called
by_owner_formatand gives each photo two attributes, owner first, format second:Now:
Entries are stored as
index, then the attributes in the order you gave them, then the primary key, and the serveranswers a query by seeking to the entries that start the same way. Owner comes first, so all of
shray's photos sit together and are cheap to find. Jpegs are spread across every owner, so there
is no single run of them to jump to. An app that needs "all jpegs regardless of owner" creates a
second index with format first.
Design decisions
The three new index commands return a new code (-3) when the server refuses them.
KVClient'sexisting methods either return data or cannot fail, so there was no way to say "the servers
said no".
CreateIndexEntry,DeleteIndexEntryandUpdateIndexEntryreturn0on successand
-3when the servers refused the request, for example indexing a record that does notexist. Anything else is passed through unchanged from
TransactionConstructor::SendRequest:-1if the request could not be sent or no reply cameback,
-2if the reply could not be parsed.-3is the only new value, and nothing else ininterface/,api/orservice/tools/kv/uses it, so no existing caller changes is affected.On the command line that looks like this (
user:999was never stored):If you would rather not add a return code, two alternatives are small changes and I am happy to
switch:
-1, which keeps the existing vocabulary but tells the caller less.These commands wait for the reply while
setdoes not.setreturns as soon as the requestleaves the client, which I confirmed by killing two of four replicas: it still returned
0although nothing could commit. That is fine for
set, which has nothing to report. Since in this design, the server can refuse these commands, so the four index methods wait for the answer just like howgetworks today.Benchmark
//benchmark/e2e:composite_key_e2e_benchmarkcompares the old way of answering a question withthe new one, against a running cluster.
The setup. Four replicas and one client proxy on a single machine, storing to LevelDB. The
benchmark writes N records, each a JSON document of a chosen size, and gives 1% of them
city = Davis. Then it times two ways of answering "which records havecity = Davis?", runseach three times and keeps the fastest:
query_by_indexcall, which is what this PR adds.Why secondary index improves with document size. Fetching everything has to carry every record across the
network, so its cost rises with the amount of data in the store. Here, we see 203 ms at 0.3 MB and 2,420 ms at
160 MB. The index query only touches the entries that match, 50 or 100 of them, so its cost barely
moves: 176 ms down to 147 ms across the same range. At small record sizes there is almost nothing
to win, because every request pays a fixed cost of roughly 150 ms (the proxy waits for 100 requests
or 100 ms before starting consensus, and a lone client pays that wait), and 0.3 MB of data is
cheap to move next to it. At 16 KB documents, the store holds 160 MB and moving all of it is the
whole cost.
On fetching the matching records. A query returns primary keys. If the app then wants the
records themselves, it does a
getper key, and each one is a separate request through consensus,about 200 ms here. For two or three matches that is fine. For 100 matches it is about 18 seconds,
which is slower than fetching the whole database in one request. That is the reason I added
with_valuesin #269. Using withvalues, the server reads the matching records itself and returns them in the same reply, so the app pays one request no matter how many records match.The trade-off.
with_valuessends back every matching record, so a query that matches most ofthe store returns most of the store. With half the records matching I measured 316 ms for fetching
everything against 228 ms with
with_values, so it still wins, but the margin narrows. Refer to theopen questions.
Tests
service/tools/kv/api_tools/composite_key_e2e_test.shstarts a cluster with LevelDB, runs 20checks against it, and prints
PASSor the first check that failed. It exits non-zero on failureso CI can run it. What it covers:
attributes, and a query that must return nothing.
ret = -3and to leave theindex exactly as it was:
photo:999, a record that was never stored,the entry but leaves the record:
getstill returns it afterwards.records.
Every write is confirmed by reading it back, because
setdoes not wait for consensus. I alsochecked the script fails when it should, by feeding it a wrong expected value and watching it
report the failure.
Note on existing behaviour
Nothing that exists changes. The CLI gains commands and flags and its existing flags and the older
positional form are untouched. The Python module keeps
getandsetand adds four functions.KVClient's existing methods are unchanged.Future work
four methods on
sdk::ResDBKVClientand one route incrow_service.cpp. Small, and it is whatmakes the feature reachable for the apps Proposal to Add Interface to Create and Retrieve CompositeKeys #180 names.
entries, the index keeps pointing at the old attributes and queries return stale results, and
nothing detects it. One possible fix is an opt-in command that writes the record and fixes its
index entries in a single atomic write, so the two cannot drift apart, and which costs one
request instead of two. I will write this up as a separate proposal.
rather than a loopback one.
Open questions
with_valuescap what it returns? Today a query matching most of the store returnsmost of the store in one reply. A limit on the number of records, or on total size, with a flag
saying the result was truncated, would make the worst case predictable. I did not add one
because I did not want to invent a policy on my own.
-3the right way to report a refusal?--attrssupport an escape? It splits on commas, so an attribute valuecontaining a comma cannot be expressed today.
Files touched
New:
service/tools/kv/api_tools/composite_key_e2e_test.sh,benchmark/e2e/composite_key_e2e_benchmark.cppand itsBUILD.Modified:
interface/kv/kv_client.{h,cpp},service/tools/kv/api_tools/kv_service_tools.cpp,api/pybind_kv_service.cpp,api/kv_operation.py.Refs #180, builds on #269