M31 · Speak gRPC, without charging anyone who does not - #63
Merged
Conversation
# Conflicts: # build.gradle.kts
The seam has been in place since the first release and has had one implementation. This is the second, and what it proves is that the public API really did carry no wire-protocol knowledge: not one line of kdrant-core changed to add a protocol. KdrantGrpc(host) returns the same QdrantClient the REST factory does, over Qdrant's Collections, Points, Snapshots and Health services on 6334. The same shared contract runs against both engines, against the same server, so the choice between them is a footprint and throughput decision rather than a feature one. The seam is wider than the protocol, and that is the interesting part. QdrantTransport was shaped by Qdrant's REST API, and eleven of its operations exist over HTTP only: telemetry, metrics, issues, snapshot recovery, snapshot transfer, and the shard-scope snapshots. Each of them throws here, naming itself and naming REST. A snapshot download that returned an empty flow would be a backup that quietly does not exist, and a KdrantException would have been the wrong type: nothing failed on the wire, and no retry will make the call work. Health is the one place the two protocols disagree and both are right. Qdrant has no gRPC /healthz, /readyz or /livez; it has the standard gRPC health service, which answers one status for the node. The three probes answer from it, and keep the REST engine's false-on-failure contract rather than throwing. Retries mirror the REST engine's, because maxRetries is a client setting and an engine that ignored it would be a behaviour difference nobody asked for. Deadlines are per call rather than per channel, because requestTimeout is documented as per attempt. A REST-only build still resolves no gRPC, no protobuf and no Netty. That was the whole argument for making this opt-in, and it is asserted rather than assumed.
The BOM exists so the modules cannot drift apart, and a module missing from it drifts by default. Importing the platform and then naming kdrant-transport-grpc without a version would resolve nothing.
The shared contract caught this on its first run against a real server, in the one test that reads a payload back from a scroll nobody configured. A scroll and a retrieve default with_payload to true on the server. The engine was translating "the caller said nothing" into an explicit enable = false, which is a different request: every scroll and every retrieve that did not ask for a payload silently came back without one. The REST engine omits the field and gets the default, which is why the two disagreed and why only a test that ran against both could see it. Absent now stays absent, on the payload selector and the vector selector alike, and a unit test asserts the fields are unset rather than false.
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.
Closes #52.
The seam has been in place since the first release and has had one implementation. This is the second,
and what it proves is that the public API really did carry no wire-protocol knowledge: not one line of
kdrant-corechanged to add a protocol.KdrantGrpc(host)returns the sameQdrantClientthe REST factory does, over Qdrant'sCollections,Points,SnapshotsandHealthservices on 6334. The shared client contract from #62 runs againstboth engines, against the same server.
Against the exit criterion — "a
kdrant-transport-grpcmodule passes the same client test suite asthe REST engine, and a REST-only build pulls in none of its dependencies":
GrpcClientContractIntegrationTestis four lines: it extendsQdrantClientContractand picks port6334. Same file the REST engine is held to.
verifyRestEngineFootprintruns on everycheckand fails if the REST engine's runtime classpathresolves anything from
io.grpc,com.google.protobuf,io.nettyorcom.google.guava. It wasconfirmed to fail by adding
grpc-stubto the REST module and watching it report the five artifactsthat arrived, then reverting. The README's footprint table rests on this, so it is checked rather
than argued.
Where the seam is wider than the protocol.
QdrantTransportwas shaped by Qdrant's REST API, andeleven of its operations exist over HTTP only: telemetry, metrics, issues,
recoverSnapshot, snapshotdownload and upload, and the five shard-scope snapshot operations. Each throws an
UnsupportedOperationExceptionnaming itself and naming REST. A snapshot download that returned anempty flow would be a backup that quietly does not exist, and a
KdrantExceptionwould have been thewrong type — nothing failed on the wire, and no retry makes the call work. The three streaming
downloads throw at the call rather than on first collect, so the failure lands where the mistake is.
Health is the one place the protocols disagree and both are right. There is no gRPC
/healthz,/readyzor/livez; there is the standard gRPC health service, which answers one status for thenode. The three probes answer from it and keep the REST engine's false-on-failure contract.
Behavioural parity. Retries mirror the REST engine's, because
maxRetriesis a client setting andan engine that ignored it would be a difference nobody asked for. Deadlines are per call rather than
per channel, because
requestTimeoutis documented as per attempt. gRPC statuses map onto the sameKdrantExceptionhierarchy, including theINVALID_ARGUMENTQdrant returns on some write paths for acollection that does not exist.
Tests. Three layers, and only the first needs Docker: the shared contract against a real Qdrant;
GrpcQdrantTransportTest, which runs the transport against fake services over gRPC's in-processtransport and asserts the messages it built and the models it read back; and focused tests for the
error mapping and for the eleven operations that cannot work. Aggregate line coverage is 80.1%.
Also updates the README's module table and footprint paragraph, and the migration guide — "Kdrant does
not speak gRPC" stopped being true.
Merge after #61 and #62.