* print categories and commands in lower case instead of capital case
* fix a bug of default user inheriting the wrong acl rules on new connections
* move keys position to be after password when printed from an acl command
* remove acl categories from context and all acl checks
* category assign,ent now assigns all the acl commands for that category to the user
* introduce modification order of acl's per user
* acl rules are now printed in the same order as in redis/valkey
* remove old user_registry_test which was part of the poc
* chore: Introduce pipeline back-pressure
Also, improve synchronization primitives and replace them with
thread-local variations.
Before the change, on my local machine with the dragonfly running with 8 threads,
`memtier_benchmark -c 10 --threads 8 --command="PING" --key-maximum 100000000 --hide-histogram --distinct-client-seed --pipeline=20 --test-time=10`
reached 10M qps with 0.327ms p99.9.
After the change, the same command showed 13.8M qps with 0.2ms p99.9
---------
Signed-off-by: Roman Gershman <roman@dragonflydb.io>
* fix(cluster-migration): Support cancelling migration right after starting it
This fixes a few small places, but most importantly it does not allow a
migration to start before both the outgoing and incoming side received
the updated config. This solves a few edge cases.
Fixes#2968
* add TODO
* fix test
* gh comments and fixes
* add comment
* change ACL DELUSER, ACL WHOAMI, and some ACL DRYRUN string/integer responses.
* change ACL GETUSER response, when the user does not exist, it should reply (nil).
* chore: clean up REPLTAKEOVER flow
1. Factor out the catchup function.
2. Simplify the flow and make the second parameters - integer.
3. Return OK if the server is already a master (and do nothing underneath).
---------
Signed-off-by: Roman Gershman <roman@dragonflydb.io>
The number of keys in an _incoming_ migration indicates how many keys
were received, while for _outgoing_ it shows the total number. Combining
the two can provide the control plane with percentage.
This slightly modified the format of the response.
Fixes#2756
fix: authorize the http connection to call DF commands
The assumption is that basic-auth already covers the authentication part.
And thanks to @sunneydev for finding the bug and providing the tests.
The tests actually uncovered another bug where we may parse partial http requests.
This one is handled by https://github.com/romange/helio/pull/243
Signed-off-by: Roman Gershman <roman@dragonflydb.io>
Send journal lsn to replica and compare the lsn value against number of records received in replica side
Signed-off-by: kostas <kostas@dragonflydb.io>
Co-authored-by: adi_holden <adi@dragonflydb.io>
* chore: preparation for basic http api
The goal is to provide very basic support for simple commands,
fancy stuff like pipelining, blocking commands won't work.
1. Added optional registration for /api handler.
2. Implemented parsing of post body.
3. Added basic formatting routine for the response. It does not cover all the commands but should suffice for
basic usage.
The API is a POST method and the body of the request should contain command arguments formatted as json array.
For example, `'["set", "foo", "bar", "ex", "100"]'`.
The response is a json object with either `result` field holding the response of the command or
`error` field containing the error message sent by the server.
See `test_http` test in tests/dragonfly/connection_test.py for more details.
* chore: cover iouring with enable_direct_fd
---------
Signed-off-by: Roman Gershman <roman@dragonflydb.io>
* feat(replication): Do not auto replicate different master
Until now, replicas would re-connect and re-replicate a master after the
master will restart. This is problematic in case the master loses its
data, which will cause the replica to flush all and lose its data as
well.
This is a breaking change though, in that whoever controls the replica
now has to explicitly issue a `REPLICAOF X Y` in order to re-establish
a connection to a new master. This is true even if the master loaded an
up to date RDB file.
It's not necessary if the replica lost connection to the master and the
master was always alive, and the connection is re-established.
Fixes#2636
* fix test
* fixes
* proxy proxy java java
* better comment
* fix comments
* replica_reconnect_on_master_restart
* proxy.close()
* feat(cluster): Add `--cluster_id` flag
This flag sets the unique ID of a node in a cluster.
It is UB (and bad) to set the same IDs to multiple nodes in the same
cluster.
If unset (default), the `master_replid` (previously known as `master_id`) is used.
Fixes#2643
Related to #2636
* gh comments
* oops - revert line removal
* fix
* replica
* disallow cluster_node_id in emulated mode
* fix replica test
1.Add back the search files to MacOs build (linker errors are fixed now).
2. Add default maxmemory argument (if not present already) when launching dragonfly process in regression tests.
Signed-off-by: Roman Gershman <roman@dragonflydb.io>
* chore: Del and NUMINCRBY use json::Path
Also, fix various protocol bugs when we sent simple string
instead of sending bulk strings.
Fixed a typo in path.cc that lead to a data race bug.
Finally, flip the flag in regression tests to start covering json::Path code
and added test coverage for the data race bug
---------
Signed-off-by: Roman Gershman <roman@dragonflydb.io>
* feat(connection): Support pipelining with Memcached
Adds support for pipelining to Memcached, enhances Memcached pytests
---------
Signed-off-by: Vladislav Oleshko <vlad@dragonflydb.io>
* upload only failed test logs
* remove printing log names for passed tests
* print slow tests with --duration
* separate regression and unit logs for CI workflow
* feat(pytest): More types for seeder
Add more types to the seeder and refactor replication test
---------
Signed-off-by: Vladislav Oleshko <vlad@dragonflydb.io>
The bug: crash when starting replica while saving
The problem: accessing the wrong allocator on snapshot class destruction as it was destructed not in the thread of the shard
The fix: call snapshot destructor when we finish snapshot on the correct thread
Signed-off-by: adi_holden <adi@dragonflydb.io>
* fix: do not migrate during connection close
Fixes#2569
Before the change we had a corner case where Dragonfly would call
OnPreMigrateThread but would not call CancelOnErrorCb because OnBreakCb has already been called
(it resets break_cb_engaged_)
On the other hand in OnPostMigrateThread we called RegisterOnErrorCb if breaker_cb_ which resulted in double registration.
This change simplifies the logic by removing break_cb_engaged_ flag since CancelOnErrorCb is safe to call if nothing is registered.
Moreover, we now skip Migrate flow if a socket is being closed.
---------
Signed-off-by: Roman Gershman <roman@dragonflydb.io>
* test(memory): Test memory accounting for all types
* slightly faster
* WIP
* working
* Document
* Update test to use DEBUG POPULATE
* Nothing much
* Working
* fix
* yaml
* explicit capture
* fix ci?
* stub tx
* feat(cluster): add tx execution in cluster_shard_migration
refactor(replication): move code that is common for cluster and
replica into a separate file, add full-sync-cut cmd
* fix(replication): Correctly replicate commands even when OOM
Before this change, OOM in shard callbacks could have led to data
inconsistency between the master and the replica. For example, commands
which mutated data on 1 shard but failed on another, like `LMOVE`.
After this change, callbacks that result in an OOM will correctly
replicate their work (none, partial or complete) to replicas.
Note that `MSET` and `MSETNX` required special handling, in that they are
the only commands that can _create_ multiple keys, and so some of them
can fail.
Fixes#2381
* fixes
* test fix
* RecordJournal
* UNDO idiotnessness
* 2 shards
* fix pytest
* feat(server): Implement `CLIENT KILL`
Currently, it supports the following syntax:
* `CLIENT KILL <addr>:<port>`
* `CLIENT KILL ID <id>`
* `CLIENT KILL ADDR <addr>:<port>`
* `CLIENT KILL LADDR <addr>:<port>`
It will not allow killing an admin-connection from a non-admin port.
There are a few parameters of `CLIENT KILL` that Redis supports but this
PR does not yet add. Let's add them as needed.
Fixes#1614
* Add tests
* fixes
fixes#2296
added a regression test that tests both policy based eviction as well as heart beat eviction.
---------
Signed-off-by: Yue Li <61070669+theyueli@users.noreply.github.com>
* feat: add SLOT-MIGRATION-STATUS cmd for source node
implements #2232
add ability using SLOT-MIGRATION-STATUS without args
to print info about all migration processes for the current node
fix#2337
The bug:
replicaof was not rejected while loading snapshot
The fix:
replicaof is allowed while server is in loading state to allow replicaof while replication in full sync mode
I now reject replicaof if the server is in loading state and it is master
Another bug fix:
allow cron snapshot if --replicaof flag was set
Signed-off-by: adi_holden <adi@dragonflydb.io>
* refactor(server): Privatize `PreUpdate()` and `PostUpdate()`
While at it:
* Make `PreUpdate()` not decrease object size
* Remove redundant leftover call to `PreUpdate()` outside `DbSlice`
* Add pytest
* Test delete leads to 0 counters
* Improve test
* fixes
* comments
1. How many transactions we processed by type
2. How many transactions we processed by width (number of unique shards).
Signed-off-by: Roman Gershman <roman@dragonflydb.io>
* feat(cluster): add command flow for slot migration process
fixes#2295
DFLYMIGRATE FLOW command was added to establish
connections for every shard replication process.
Slow serialization step is the separate issue so
for now only eof_token is sent for reply to
DFLYMIGRATE FLOW command.
Expected state for START-SLOT-MIGRATION is FULL_SYNC now.
* feat: DispatchTracker
Use a DispatchTracker to track ongoing dispatches for commands that change global state
---------
Signed-off-by: Vladislav Oleshko <vlad@dragonflydb.io>
fix: eliminate the redundant string copy in SendMGetResponse
Also, allow selectively create DflyInstance in pytests that is attached to
an existing dragonfly port, created outside of tests.
Signed-off-by: Roman Gershman <roman@dragonflydb.io>
The DF version is being unparseable by Memcached::getVersion() that expects n.n.n string.
Change the version to emulate the old memcached server.
The DF version can still be fetched via Memcached::getStats() function.
Signed-off-by: Roman Gershman <roman@dragonflydb.io>
* chore: add more states to client connections
* fix: clear pipelined messages before close
* fix: skip same thread on backpressure
---------
Signed-off-by: Vladislav Oleshko <vlad@dragonflydb.io>
Co-authored-by: Roman Gershman <roman@dragonflydb.io>
* fix(server): client pause fix on pipeline squash
allow squashing commands on pause
move await on client pause inside InvokeCommand - this way all flows of command invoke will read pause state
Signed-off-by: adi_holden <adi@dragonflydb.io>
This PR introduces a test case for TLS with `ca_dir`. First, we
did not have any tests for this case. Second, using `ca_dir` requires
to call `c_rehash` on the directory before it is loaded by DF. We
did not have this use case anywhere and therefore we thought there was
a bug when we used `ca_dir` only to find out that we need to call
`c_rehash` on the directory before we load the certificates. Now,
both a test and a use case are properly documented
* add missing test for ca_dir
* use rehash to properly show how to load ca directories instead of
files
Regression test sometimes fails because for a short period of time after `wait_available_async()` returns, the result of `ROLE` could still be different from `stable_sync`
[Failure example](https://github.com/dragonflydb/dragonfly/actions/runs/6726461923/job/18282759612#step:6:1863)
We change our state from `LOADING` to `ACTIVE` [here](d08d7f13b4/src/server/replica.cc (L426)), but then we change the sync state 2 times:
1. `!R_SYNCING` [here](d08d7f13b4/src/server/replica.cc (L427C28-L427C37))
2. And only later to `R_SYNC_OK` (meaning `stable_sync`) [here](d08d7f13b4/src/server/replica.cc (L221))
This is easy to reproduce by adding a sleep right after the set of state to `ACTIVE`, either before or after the flipping of `R_SYNCING` (with different returned states).
BTW without that added sleep I was not able to reproduce, having tried 1000s of times in various configurations.
We could change the order of things such that we first change `state_mask_` and only then switch state from `LOADING` to `ACTIVE` (which is probably the right thing to do), but that would require a subtle refactor, as we change these in a couple of places.
But we should keep in mind that this has no effect on users. So a simple sleep on the test side should fix this fairly well.
* chore: help users to fix a common mistake of setting quotes in the flagfile
Specifically, the confusion is often around the cron expression.
---------
Signed-off-by: Roman Gershman <roman@dragonflydb.io>
* add a python script to print the most recent log
* if CI timeouts, print the most recent log
* replace global timeout with timeout command
* upload all logs on failure()
* print uid + port + the log files for each df instance
* requirepass also updates ACL default user password
* update config set requirepass to include the new behaviour
* add tests
* fix non existent default user when loading empty files
* Update Http auth with username default instead of user
* skip auth for /metrics page
* add/improve tests
* fix a bug with admin port requiring auth on http even if nopass was set
* update helio ref
* update listener class to contain its respective Role
* fix http init to only include admin and main listener
* The issue was similar with test `network_disconnect_small_buffers` but this time the debug build could be slow enough for the replication to not finish. As a consequence, by the time the test reached the assertion, the log did not contain the expected output.
* chore(regression): test bptree on regression pytests
1. stop passing the flag use_zset_tree as it is true on default
2. fix ci test to run replication tests
3. change replication tests seeder to sometimes add more than 128 values
to zset to test the pbtree impl
Signed-off-by: adi_holden <adi@dragonflydb.io>
* chore: Add a context manager to DflyInstance so we don't forget to close
them.
* Update tests/dragonfly/config_test.py
Co-authored-by: Roman Gershman <roman@dragonflydb.io>
Signed-off-by: Roy Jacobson <roi.jacobson1@gmail.com>
---------
Signed-off-by: Roy Jacobson <roi.jacobson1@gmail.com>
Co-authored-by: Roman Gershman <roman@dragonflydb.io>
* flags from env variables
* querying environment vars
* remove includes
* refactor
* exit for unknown flag with DFLY_ prefix
* reflecting change in the test
* better tests
* refactor + new test case
* refactor test with inner class
* refactor
* revert back test flags as it might affect ci/cd
* fixing flags
* refactor
* remove includes
* refactor
* move error handling tests from regression to unit
* move ACL LIST regression to unit test
* move AUTH regression to unit test
* move ACL WHOAMI regression to unit test
* add unit tests for SETUSER/DELUSER (so they run on every PR)
* add unit tests for all ACL categories
There was a bug on updates of the acl categories when squashing was used. Basically, the parent context could be accessed in parallel by the "stub" contexts causing a dreaded data race on the update.
This is fixed by adding a new AclUpdateMessage at the front of the dispatch queue of the connection.
1. No logic was changed during refactoring.
2. Flipped the flag to run regression tests for now own with zset_tree=on
Signed-off-by: Roman Gershman <roman@dragonflydb.io>
* fix DispatchCommand error reporting when memcached protocol is used (one example is when we use SET command on the replica -- previously we crashed now we properly report an error)
* SendError(ErrorReply) moved to SinkReplyBuilder from RedisReplyBuilder
* SendError(OpStatus) moved to SinkReplyBuilder from RedisReplyBuilder
* added tests for SendError(ErrorReply) in RedisReplyBuilder
* feat: implement CONFIG GET command
The command returns all the matched arguments and their current values.
In addition, this PR adds mutability semantics to each config - whether it can be
changed at runtime.
Fixes#1700
Signed-off-by: Roman Gershman <roman@dragonflydb.io>
---------
Signed-off-by: Roman Gershman <roman@dragonflydb.io>
* feat(server): Support limiting the number of open connections.
* * Update helio after the small fix was merged to master
* Don't limit admin connections (and add a test case)
* Resolve CR comments
1. If the first request sent to the connection is large (2kb or more)
Dragonfly was closing the connection.
2. Changed server side error reporting according to memcache protocol:
https://github.com/memcached/memcached/blob/master/doc/protocol.txt#L172
3. Fixed the wrong casting in DispatchCommand.
4. Remove practically unused code that translated opstatus to strings.
Signed-off-by: Roman Gershman <roman@dragonflydb.io>
