The old tests used partial error string matching, with the whole error
message! So when I added more to the error message, the fixture tests
didn't fail.
This changes the tests to match the full string, and validate the
timestamps are included.
Include the minimum acceptable timestamp (the retention bound) and the
observed timestamp that exceeds this bound in the retention enforcement
write error response.
The replication flag defines the total number of copies of each write -
slightly less confusing than the additional copies it was previously,
and matches with the actual code.
Changes the UpstreamSnapshot to be suitable for concurrent use. This
type contains the core logic to enable a caller to uphold the
responsibility of ensuring replicated writes land on distinct ingesters
in the presence of concurrent replication.
The clients within the snapshot are returned to at most one concurrent
caller at a time, by tracking the state of each client as a FSM:
┌────────────────┐
┌─▶│ Available │
│ └────────────────┘
│ │
drop next()
│ │
│ ▼
│ ┌────────────────┐
└──│ Yielded │
└────────────────┘
│
remove
│
▼
┌────────────────┐
│ Used │
└────────────────┘
Once a client has been yielded it will not be yielded again until it is
dropped (transitioning the FSM from "yielded" to "available" again,
returning it to the candidate pool of clients) or removed (transitioning
to "used", permanently preventing it from being yielded to another
caller).
Changes then UpstreamSnapshot to return owned clients, instead of
references to those clients.
This will allow the snapshot to have a 'static lifetime, suitable for
use across tasks.
Because the number of candidate upstreams is checked to exceed the
number of desired data copies before starting the write loop, and
because the parallelism of the write loop matches the number of desired
data copies, it's not possible for any thread to observe an empty
snapshot.
This commit removes the unreachable error condition for clarity.
Adds a property-based test of the RPC write handler's replication logic,
ensuring:
1. If the number of healthy upstreams is 0, NoHealthyUpstreams is
returned and no requests are attempted.
2. Given N healthy upstreams (> 0) and a replication factor of R:
if N < R, "not enough replicas" is returned and no requests are
attempted.
3. Upstreams that return an error are retried until the entire
write succeeds or times out.
4. Writes are replicated to R distinct upstreams successfully, or
an error is returned.
5. One an upstream write is ack'd as successful, it is never
requested again.
6. An upstream reporting as unhealthy at the start of the write is
never requested (excluding probe requests).
These properties describe a mixture of invariants (don't replicate your
two copies of a write to the same ingester) and expected behaviour of
the replication logic (optimisations like "don't try writes when you
already know they'll fail").
This passes for the single-threaded replication logic used at the time
of this commit, and will be used to validate correctness of a concurrent
replication implementation - a concurrent approach should uphold these
properties the same way a single-threaded implementation does.
Renames NoUpstreams -> NoHealthyUpstreams as it's confusing because we
also have "not enough replicas" which could be no upstreams? This has a
slightly clearer meaning.
Refactors the From<BtreeMap> impl that accepted a &str name for
ColumnsByName construction, instead allowing only the owned String, and
updating the test that makes use of it appropriately.
So that the different kinds aren't mixed up. Also extracts the logic
having to do with which template takes precedence onto the
PartitionTemplate type itself.
This commit splits out the RPC-request-centric errors in RpcWriteError
into their own RpcWriteClientError type.
This improves the separation of concerns - an RpcWriteError comes from
the RPC write handler, whereas an RpcWriteClientError comes from an
underlying client. It's definitely less confusing!
Still insert them into the database and associate them with namespaces,
but don't ever query them back out.
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
* refactor(authz): move extract_header_token into authz
Move the extract_header_token method into the authz package so that
it can be shared by the query path. The method is renamed to reflect
the fact that it can now also extract a token from gRPC metadata.
The extract_token function is now a little more generic to allow
it to be used with HTTP header values and gRPC metadata values.
* feat(service_grpc_flight): JDBC compatible Handshake
While testing some JDBC based clients we found that some, Tableau
in this case, cannot be configured with authoriztion tokens. In
these cases we need to be able to support username/password. The
approach taken is to ignore the username and make the token the
password. This is the same approach being taken throughout the
product.
To facilitate this the Flight RPC Handshake command has been extended
to look for Basic authorization credentials and respond with the
appropriate Bearer authorization header.
While adding end-to-end tests the subprocess commands were causing
a deadlock. These have been changed to using the tonic::process
module.
There are also some small changes to the JDBC test application where
the hardcoded values were clashing with the authorization parameters.
* fix: lint
* chore: apply suggestions from code review
Co-authored-by: Andrew Lamb <alamb@influxdata.com>
* chore: review suggestion
---------
Co-authored-by: Andrew Lamb <alamb@influxdata.com>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
This commit adds a randomised property test, that compares the results
of the new namespace cache schema merging (#7555) with a known-good
stdlib HashSet union (the cache implementation is effectively a more
specialised set union operation).
This property test also validates the "last writer wins" semantics for
other, non-schema data within the namespace.
Additionally the ChangeSet values returned over a pair of updates are
asserted to reflect the actual values added to the cache (but not each
call individually) to ensure accurate metrics are reported.
Dom Dwyer
dependabot[bot]
Andrew Lamb
wiedld
wiedld
Carol (Nichols || Goulding)
Kaya Gökalp
Carol (Nichols || Goulding)
Martin Hilton
Fraser Savage