shotover / shotover-proxy Goto Github PK
View Code? Open in Web Editor NEWL7 data-layer proxy
Home Page: https://docs.shotover.io
License: Apache License 2.0
L7 data-layer proxy
Home Page: https://docs.shotover.io
License: Apache License 2.0
Using redis-bench with -P 1 with new Shotover and below single cluster topology results in this error:
thread 'RPProxy-Thread' panicked at 'index out of bounds: the len is 2 but the index is 2', /home/rust/.cargo/git/checkouts/redis-rs-4c7ae24e54934319/e315517/src/cmd.rs:631:17
Topo file
sources:
redis_prod:
Redis:
batch_size_hint: 10
listen_addr: "127.0.0.1:6379"
chain_config:
redis_chain:
- RedisCluster:
first_contact_points: ["redis://3.211.142.207"]
named_topics:
See https://redis.io/topics/protocol
Currently mostly works, but needed to test more
See https://redis.io/topics/cluster-spec for details:
If the hash slot is served by the node, the query is simply processed, otherwise the node will check its internal hash slot to node map, and will reply to the client with a MOVED error, like in the following example:
Example:
GET x
-MOVED 3999 127.0.0.1:6381
The error includes the hash slot of the key (3999) and the ip:port of the instance that can serve the query. The client needs to reissue the query to the specified node's IP address and port. Note that even if the client waits a long time before reissuing the query, and in the meantime the cluster configuration changed, the destination node will reply again with a MOVED error if the hash slot 3999 is now served by another node. The same happens if the contacted node had no updated information.
So while from the point of view of the cluster nodes are identified by IDs we try to simplify our interface with the client just exposing a map between hash slots and Redis nodes identified by IP:port pairs.
There may be differences in the way that a client (with cluster awareness) handles load balancing vs. the way that shotover handles load balancing. For example, I've noticed that redis-bench with cluster mode enabled results in a set load on 2 master nodes, and 2 replica nodes (for a 3 master, 3 replica cluster). It's odd that it doesn't use all 3 master nodes.
Same client (without cluster mode) via shotover results in different behaviour, with sets, 1 master and 1 replica are used, with gets, 1 master is used. Can we make shotover cluster load balancing behaviour configurable (many clients support either master only, or master and replica load balancing), allow all master nodes to be used for better load distributed (for sets), and output confirmation about what nodes are being used (and how) for load balancing on shotover startup.
We need to support TLS based TCP connections to support various encrypted database protocols that make use of this.
Our first target will be compatibility with Redis and Cassandra TLS.
The following is TBD:
-> Certificate store format
-> Do we need to support certificate authentication (e.g. identity by cert)
-> Do we need to enforce trust (e.g. client cert signed by a cert shotover trusts).
Currently scatter/gather (what's used to support multi-region / active-active) will ignore the results of left over requests when the number of required responses is < total queried nodes (e.g. a quorum).
The next request that hits that chain will get the old response that has been sitting in the socket buffer.
Redis doesn't support request id's (so the driver can recieve out of order responses). So need a way to track responses we need to discard.
See https://redis.io/topics/cluster-spec
-ASK is like moved... but is just for that next request, it is a temporary indication of key ownership (so don't update SLOT mappings if/when they exist).
It's a bit weird that we have to pass a reference to the chain via &t, when we could probably hide that behind a method in the transform itself?
Maybe not
But worth looking a simplifying the transform chain
Evaluate tower.rs
E.g what linkerd2 uses
https://github.com/linkerd/linkerd2-proxy
shotover is an application not a library so we should check in the Cargo.lock as per https://doc.rust-lang.org/cargo/faq.html#why-do-binaries-have-cargolock-in-version-control-but-not-libraries
Using either something like:
Create a Transform that will pass a Wrapper struct into a user defined WASM function, and also allow the user to call call_next_transform
on it to run subsequent transforms.
Initially a basic implementation of the Transform config will look something like:
- WASMTransform:
- path: myfunction.wasm
- name: Func
We want to support the ability to perform a hot reload of shotover (similar to the way HAProxy and Envoy proxy do), where we can "restart" the executable such that:
To acheive such an ability, we would like to introduce the ability for shotover to start in "handover" mode, where it will receive existing tcp socket file descriptions from a currently running shotover process and gradually take over them.
This method of socket handover between processes is best shown in the following repo: https://github.com/benbromhead/hot-reload-example. This repo also contains links to prior art and some good blog posts.
This is likely going to be a change that requires a few changes to shotover. Such as:
Currently shotover doesn't support redis password authentication with the new Redis Cluster transform implementation.
We will want to support both RESP2 and RESP3 authentication mechanisms. See https://redis.io/commands/auth
The following section is a list of tasks / changes required to support the redis caching, authentication for redis and general development on shotover.
new
function that is also async.There are some areas and some transforms that just straight up swallow or log an error but don't do anything sensible with it.
In writing a set of integration tests. Cassandra-rs will fail to connect to shotover configured with a Cassandra source, while cqlsh will connect succesfully.
After a quick look at wireshark, it appears that the cassandra-rs driver (a wrapper around the c/c++ datastax one) tries a few unsupported protocol version, which fail or get downgraded. Cassandra itself wont shutdown the tcp connection, whereas shotover will causeing the driver to error out.
Given Cassandra supports this behaviour (as broken as it seems), shotover needs to support this. Protocol version errors shouldn't cause us to drop the connection.
Support upgrading binary without dropping connections
Support config change without process restart
Tests should panic instead of returning Result.
If we panic we get a traceback that includes the exact line number that the error occurred at.
Whereas that information is lost if we return the Result all the way up to the test function.
I'm interested to hear why it was done this way, maybe there are some positives to it I cant think of?
With the topology file below (2 Redis clusters), using redis-cli connected to shotover and simple get key operation results in the following error:
Aug 27 02:08:57.754 DEBUG shotover_proxy::transforms::distributed::tuneable_consistency_scatter:
None
Aug 27 02:09:35.141 DEBUG shotover_proxy::transforms::redis_transforms::timestamp_tagging:
Generated eval script for timestamp: return {redis.call('get','a'),redis.call('OBJECT', 'IDLETIME', KEYS[1])}
Aug 27 02:09:35.141 DEBUG shotover_proxy::transforms::redis_transforms::timestamp_tagging: tagging transform got Err(Redis Cluster transform did not have enough information to build a request)
Aug 27 02:09:35.141 DEBUG shotover_proxy::transforms::redis_transforms::timestamp_tagging: response after trying to unwrap -> Err(Redis Cluster transform did not have enough information to build a request)
Aug 27 02:09:35.142 DEBUG shotover_proxy::transforms::distributed::tuneable_consistency_scatter: Got 0, needed 1
sources:
redis_prod:
Redis:
batch_size_hint: 10
listen_addr: "127.0.0.1:6379"
chain_config:
redis_chain:
- ConsistentScatter:
write_consistency: 1
read_consistency: 1
route_map:
one:
- RedisTimestampTagger
- RedisCluster:
first_contact_points: ["redis://3.211.142.207"]
two:
- RedisTimestampTagger
- RedisCluster:
first_contact_points: ["redis://34.231.231.80"]
named_topics:
Write some
Continuing discussion from #109 (comment)
Kuangda made a good point:
Question is, should we expect the project root to be a somewhat usable working directory for running the application?
In the (managed service) Docker container, those files are located in /opt/shotover-proxy/config/.
On second thought, it might be preferable to leave the files and add some docs instead.
If we do keep the config folder for that reason, then we will need to add a test + docker config for it so that CI can ensure the config/ folder is always valid.
Given that complication maybe its better to just document that the user should copy an examples topology.yaml
file to config/topology.yaml
from the example most relevant to them, maybe recommend redis-passthrough if they dont know what they want.
The advantages here are:
config/
folderSo I am proposing the following changes:
examples/*/config.yaml
to examples/*/topology.yaml
config/topology.yaml
file.cargo run
in the root directory.examples + integration tests + cargo run
will all continue to use the config file at config/config.yaml
SLOT discovery will ensure key routing to the right node instead of relying on the MOVED mechanism.
Support the ability to test codecs / protocol implementations by replaying a pcap file and comparing to the expected serialized version.
The current implementation of integration tests relies on a thread::sleep(4s)
to avoid race conditions on certain configurations but I think we can do better than this.
Removing this sleep allows tests that dont use redis clustering to pass such as test_pass_through.
However test cases that use clustering blow up due to a race condition, the issue is follows:
CLUSTER SLOTS
will return empty results.CLUSTER SLOTS
will return a list of all the instances in the cluster.If shotover is started while we are still at 2 then shotover will fail to startup as it needs to get a list of instances in order to startup.
I am thinking we should handle a retry for this in the shotover application instead of the tests as this seems like a likely scenario in production.
Shotover does update its internal slot map when it hits issues so accidentally querying too early and getting an incomplete list of cluster instances should not cause issues.
@benbromhead thoughts?
I have written generic logic to block in DockerCompose::new until all ports exposed by the docker compose file are accepting connections.
But on my machine this retry logic doesn't ever have to actually retry.
So I suspect the docker-compose up command is actually blocking until the containers are completely up.
If this is the case I'll abandon this idea for now as it would be unnecessary.
If we do find that we are hitting race conditions where the containerized application isnt accepting connections yet, then we can investigate adding it at that point.
use compose_yml::v2::{File, Ports, Protocol};
use std::net::TcpStream;
impl DockerCompose {
pub fn new(file_path: &str) -> Self {
DockerCompose::clean_up(file_path).unwrap();
info!("bringing up docker compose {}", file_path);
run_command("docker-compose", &["-f", file_path, "up", "-d"]).unwrap();
let compose = File::read_from_path(file_path).unwrap();
while !DockerCompose::is_compose_ready(&compose) {
thread::sleep(time::Duration::from_millis(100));
}
DockerCompose {
file_path: file_path.to_string(),
}
}
fn is_compose_ready(compose: &File) -> bool {
compose.services.values().all(|service| {
service.ports.iter().all(|port| {
let port = port
.value()
.expect("dont use env var interpolation in examples");
if let Protocol::Tcp = port.protocol {
match port.host_ports {
Some(Ports::Port(port)) => {
println!("{}", port);
TcpStream::connect(("localhost", port)).is_ok()
}
_ => {
unimplemented!("Figure out how to handle this when we actually need it")
}
}
} else {
true
}
})
})
}
}
Full implementation: rukai@a808c86
Update dependencies to their latest versions.
e.g. I can see that the metrics
dep is 0.12 but the latest is 0.17
Once the repo goes public we can track out of date dependencies on https://deps.rs/ but for now I think we just need to manually check the version ourselves.
Currently there are vestiges of support for redis pub/sub. This needs a rethink about how the proxy will support pub/sub traffic flows as these largely don't follow a request/response model which the proxy kind of expects.
I thought I had resolved tests not reporting shotover Err in #126 but apparently not.
There are two problems we want to resolve here:
run_chains
returns Err the error is not displayed in tests.We need to do one of the following:
See timber.io example for inspiration:
https://github.com/timberio/vector/pull/2006/files
Timber.io RFC
https://github.com/timberio/vector/blob/41d90b7df1995429d179c89a4f1c176d72a263f1/rfcs/2020-04-15-wasm-plugins.md
Lucet example
https://bytecodealliance.github.io/lucet/
Added pipelines to my Jedis client, worked ok directly, but via shotover I get this error:
redis.clients.jedis.exceptions.JedisConnectionException: java.net.SocketTimeoutException: Read timed out
sources:
redis_prod:
Redis:
batch_size_hint: 10
listen_addr: "127.0.0.1:6379"
chain_config:
redis_chain:
- RedisCluster:
first_contact_points: ["redis://3.211.142.207"]
named_topics:
I first noticed this when testing something else on a mirroring enabled cluster.
The basic summary is as we turn up the number of operations (-n), the performance degrades pretty significantly, as seen below.
~ $ sudo docker exec -ti redis redis-benchmark -p 6378 -n 100000 -t set -r 100000000 -P 8 -c 50 --threads 10 -q
SET: 62427.95 requests per second
~ $ sudo docker exec -ti redis redis-benchmark -p 6378 -n 110000 -t set -r 100000000 -P 8 -c 50 --threads 10 -q
SET: 46743.10 requests per second
~ $ sudo docker exec -ti redis redis-benchmark -p 6378 -n 150000 -t set -r 100000000 -P 8 -c 50 --threads 10 -q
SET: 27566.50 requests per second
Environment:
Running locally on 1 node of a 3 node t3.medium cluster.
Topology.yaml
sources:
redis_prod:
Redis: {batch_size_hint: 4, listen_addr: '0.0.0.0:6378', connection_limit: 20000,
hard_connection_limit: true}
chain_config:
redis_chain:
- MPSCTee:
behavior: IGNORE
buffer_size: 10000
chain:
- QueryTypeFilter: {filter: Read}
- Coalesce:
max_behavior: {COUNT: 2000}
- MPSCForwarder:
buffer_size: 100
async_mode: true
timeout_micros: 10000
chain:
- QueryCounter: {name: DR chain}
- PoolConnections:
name: RedisCluster-DR-subchain
parallelism: 256
chain:
- RedisCluster:
first_contact_points: ['34.211.224.239:6379']
- QueryCounter: {name: Main chain}
- PoolConnections:
name: RedisCluster-Main-subchain
parallelism: 512
chain:
- RedisCluster:
first_contact_points: ['34.204.221.1:6379']
named_topics: {example: 10}
source_to_chain_mapping: {redis_prod: redis_chain}
While testing shotover I noticed very low throughputs with redis-bench c.f. direct use. Realised I was using an older version of redis-bench (2019) which "worked" with shotover, so reduced throughput was only observable difference. Ungrading to current redis-bench fixed the problem. Potentially therefore older Redis clients may exhibit similar problems with no errors?
Using redis-bench as test client with shover and a 6 node Instaclustr Redis cluster (3 masters, 3 replicas), conducted a test involving killing one of the Redis master threads (with cli shutdown command, which results in the replica taking over as master, and the master becoming the replica after a short delay).
With redis-bench directly connected to the cluster it stops with an error.
Connected via shotover there's no redis-bench error and it keeps working, but at a significantly slower throughput - from 12,000 to 2,000 sets/s. Killing and starting redis-bench again solves the throughput problem.
Shotover spits out the following error (xN times):
Aug 24 02:02:18.200 ERROR shotover_proxy::server: chain processing error - Got connection error with cluster Connection refused (os error 111)
Benchmark redis-cluster perf against https://github.com/RedisLabs/redis-cluster-proxy
Implement better criterion microbenches
Currently shotover can support a basic example form of row level (or cell level) encryption. This issue will track the outstanding tasks required to bring it up to a production grade quality.
Kuangda brought up the topic of rustfmt to me and I thought I would move the discussion here.
For now we dont need rustfmt as we are few developers and I personally prefer manually formatted code over rustfmt code.
However as the project grows I think we will find reviewing formatting time consuming and a mismatch of styles can occur.
So I think at some point we will want to run rustfmt across the entire repo and enforce it via CI.
So should we rustfmt'ify the code base now while PRs are low?
Or should we hold off on the decision till later?
TestContext
has a very general sounding name but is actually only used for creating redis connections to shotover.
Lets move all of its functionality into the ShotoverManager type.
So the API I'm thinking of would look something like this in shotover-proxy/tests/helpers/mod.rs
impl ShotoverManager {
pub fn redis_connection() -> redis::Connection {
unimplemented!("take implementation from TestContext::new_internal")
}
// In the future we will also add methods like cassandra_connection etc
}
We can also delete shotover-proxy/tests/redis_int_tests/support.rs
as encode_value
in support.rs
is unused.
The test_* helper methods in basic_driver_tests.rs will be refactored to take a redis::Connection
as an argument. (or a ShotoverRunner
if they really need to make a connection per test case for some reason)
Currently shotover-proxy/tests/helpers/mod.rs
is duplicated at shotover-proxy/benches/helpers.rs
to avoid circular dependencies between shotover-proxy
and test-helpers
crates.
Lets try and fix that in this PR.
I think the best solution is to set a custom module path in redis_benches.rs like this:
#[path="../tests/helpers/mod.rs"]
mod helpers;
A little hacky but seems better than the alternatives.
There has been a lot of work/discussion towards scriptable or plugin-driven transforms.
But this sounds like it could be the wrong approach.
The transforms have the same requirements as shotover itself which needs to be:
The perfect candidate for this is rust, we obviously believe that because shotover itself is written in rust.
Even so we could still use rust compiled to wasm or rust dylib plugins.
That way developers can use rust or if they really really want they can use another language.
But doing so has several disadvantages:
So how can we give shotover the flexibility it needs without plugins/wasm?
We can expose shotover as a library crate exposing a builder pattern API.
Then the user of shotover implements their own binary crate using this shotover library.
e.g. They could run vanilla shotover with just the following
fn main() {
Shotover::new().run();
}
This would be a very batteries included approach. run
would be responsible not just for running shotover but it would also start the async runtime, read the config from disk, start the logger etc.
We could still provide a binary release of shotover by compiling this simple example. This binary release could do everything except implement custom transforms.
Then to extend shotover with their own transforms, developers implement a Transform
trait and register it like this:
struct MyTransform { }
impl Transform for MyTransform {
...
}
fn main() {
Shotover::new()
.register_transform(MyTransform::new())
.run();
}
As a bonus we could allow bundling config into the binary like this:
fn main() {
Shotover::new()
// config guaranteed to exist at compile time
.set_config(include_str!("config.yaml"));
.run();
}
or we could even go so far as to have the config constructed in our rust builder pattern instead of yaml:
fn main() {
Shotover::new()
// config structure gauranteed to be correct at compile time
.add_source(Source { name: "foo", batch_size_hint: 100, listen_addr: "127.0.0.1:6379");
.add_chain(Config {
name: "bar",
sources: vec!("foo"),
transforms: vec!(
MyTransform::new(),
StandardTransform::new(TransformConfig { contact_points: vec!("127.0.0.1:2920", "127.0.0.1:2921") })
)
});
.run();
}
This particular idea is a little out there. I think just having a yaml file is more readable.
An example of this sort of "rust API instead of a binary" approach can be seen in a personal project I built, which provides a rust API for compiling gameboy asm: https://github.com/rukai/ggbasm
And of course rust webservers all use a similar approach. e.g. https://github.com/SergioBenitez/Rocket
Like the redis integration tests, we need to implement a set of Cassandra tests from a chosen rust Cassandra driver.
This might be to just pass auth on to all servers? Or auth to the proxy itself?
Currently only redis_codec_destination uses it to fast forward messages it misses.
This is really only an issue when used in conjunction with consistent scatter transform (right now the only thing that ignores upstream results and won't even poll a future for it).
We could probably scrap the whole message clock thing if we moved responsibility for draining ignored futures back into consistent scatter, this would like be the best course of action.
A declarative, efficient, and flexible JavaScript library for building user interfaces.
๐ Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. ๐๐๐
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google โค๏ธ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.