**Background**
In v1.21.0 we introduced support for `--announce_ip` for replicas to
announce their public IP addresses.
Like Valkey, this uses `REPLCONF IP-ADDRESS` to announce their IP
address.
**The issue**
Older Dragonfly releases (<1.21) did not support this feature. The
master side simply returned an error for such `REPLCONF` attempts,
however the replica code failed the replication, resulting in
incompatible versions.
**The fix**
The fix is simple, just log an error if the master did not respect
`REPLCONF IP-ADDRESS`. We can make this non-optional in the future
again.
However, in addition, I added a regression test to make sure we are
backwards compatible with v1.19.2. We'll bump this up every once in a
while.
* feat: Allow pre-declaring Lua SHAs to run with undeclared keys
By using `--lua_undeclared_keys_shas=SHA,SHA,SHA` users can now specify
which scripts should run globally (undeclared keys) without explicit
support from the scripts themselves.
Fixes#2442
* chore: add timeout fo replication sockets
Master will stop the replication flow if writes could not progress for more than K millis.
---------
Signed-off-by: Roman Gershman <roman@dragonflydb.io>
Signed-off-by: Roman Gershman <romange@gmail.com>
Co-authored-by: Shahar Mike <chakaz@users.noreply.github.com>
* chore: change how we track memory_budget during evictions
We compared memory_budget vs 0 before inserting a new item in DbSlice,
and retired cool pages if we are low on memory.
The problem - when we decide whether we allow growing a table, we estimate the possible object size increase due to the future table growth.
And the memory check described before was not consistent with the actual logic that rejected the insertion.
Moreover, the memory_budget tracking interaction with EvictionPolicy was over-complicated: we passed the memory_budget counter to the evp object
and then read it back, even though evp did not track object deletions memory impact during objects evictions.
Now, we remove the responsibility from evp to update memory_budget_ so it's solely updated by DbSlice.
We also update memory_budget_ during deletions, and when we pass it to evp, we add cool memory size as potential memory resource to avoid
rejections in case we have lots of cool memory.
Fixes#3456
---------
Signed-off-by: Roman Gershman <roman@dragonflydb.io>
os.remove(LAST_LOGS) might throw an exception if the file does not exist which we do not handle. Wrap it in try/catch block
* wrap in try/catch os.remove
---------
Signed-off-by: kostas <kostas@dragonflydb.io>
The quicklist.* files are mostly equal to the versions at commit 0fc43edc6
Also, removed some unused code.
Signed-off-by: Roman Gershman <roman@dragonflydb.io>
The env variables exported when regression tests timeout are not working properly and the if statement on the action step Print last log on timeout would fail to read and upload the files set in /tmp/last_log_file.txt. Furthermore, another problem is the job.timeout argument that kills the whole job/matrix before the upload log step has a chance to run. For that, we need manual timeouts on the workflow similar to what we do in regression tests action.
* remove print last log on timeout action step
* copy the logs on timeout directly within the timeout step
* replace global timeout on CI workflow with timeout command per step
---------
Signed-off-by: kostas <kostas@dragonflydb.io>
Before that PAUSE paused the reconnection reconciler flow,
now it also stops the ongoing full sync replication if such exists.
In addition, this PR applies some clean-ups and removes redundant code
Signed-off-by: Roman Gershman <roman@dragonflydb.io>
* chore: simplify master replication cancelation interface
Before that CancelReplication did too many things, moreover,
we had StopReplication that did the same.
This PR moves CancelReplication under ReplicaInfo struct,
and reduces code duplication around this change.
Signed-off-by: Roman Gershman <roman@dragonflydb.io>
* Update src/server/dflycmd.cc
Co-authored-by: Shahar Mike <chakaz@users.noreply.github.com>
Signed-off-by: Roman Gershman <romange@gmail.com>
---------
Signed-off-by: Roman Gershman <roman@dragonflydb.io>
Signed-off-by: Roman Gershman <romange@gmail.com>
Co-authored-by: Shahar Mike <chakaz@users.noreply.github.com>
* chore: reorganize EngineShard::Heartbeat
1. Simplify CacheStats by using accessorts directly provided by DbSlice
2. Separate eviction for tiering as tiering can be done on replica.
---------
Signed-off-by: Roman Gershman <roman@dragonflydb.io>
* chore: improve replication locks
Allow non-exclusive, read-only access to Dfly::ReplicaInfo structure.
The most important change is in DflyCmd::CancelReplication, where before
it has locked ReplicaInfo mutex and then continued with locking the global mutex.
It is dangerous because most operation lock them in the opposite order.
Also rename ambigous GetReplicaInfo accessors to clearer names.
Signed-off-by: Roman Gershman <roman@dragonflydb.io>
* chore: comments
* chore: comments
---------
Signed-off-by: Roman Gershman <roman@dragonflydb.io>
* feat: Support `replica-announce-ip`/`port`
Before this PR, we only supported `cluster_announce_ip`.
It's basically the same feature, but used for cluster announcements
instead of replication.
This PR adds support for `replica-announce-ip` and
`replica-announce-port`, which can be set via new flags `--announce_ip=`
and `--announce_port=`. These flags apply to both cluster and replica
announcements.
Tested via running Sentinel, and making sure it is able to connect to
announced ip+port, while it can't connect to announced false /
unavailable ip+port.
Note: this PR deprecates `--cluster_announce_ip`, but continues to
support it. We will remove it in a future version.
Fixes#3380
* fix failing test
* destructure
Now unit tests will run the same Hearbeat fiber like in prod.
The whole feature was redundant, with just few explicit settings of maxmemory_limit
I succeeeded to make all unit tests pass.
In addition, this change allows passing a global handler that is called by heartbeat from a single thread.
This is not used yet - preparation for the next PR to break hung up replication connections on a master.
Finally, this change has some non-functional clean-ups and warning fixes to improve code quality.
Signed-off-by: Roman Gershman <roman@dragonflydb.io>
Before - sending 200K items requires more than 12K send calls.
Now - requires less than 2K calls. Latency also went down though not by x6.
Signed-off-by: Roman Gershman <roman@dragonflydb.io>
* chore: expose metric that shows how many task submitters are blocked
This should help us in identifying deadlocks quickly.
---------
Signed-off-by: Roman Gershman <roman@dragonflydb.io>
* feat: Support non-root paths for json.merge
Pass path argument and rewrite the JSON.MERGE code similar to OpToggle
or other mutating functions. Currently works only with --experimental_flat_json=false.
Signed-off-by: Roman Gershman <roman@dragonflydb.io>
* chore: comments
---------
Signed-off-by: Roman Gershman <roman@dragonflydb.io>
1. Add background offloading stats
2. remove direct_fd override - helio is already updated with default=false, so it's not needed anymore.
3. remove redundant tiered_storage_memory_margin flag
Signed-off-by: Roman Gershman <roman@dragonflydb.io>
json.merge would throw an exception when the json object did not contain the element to replace because RecursiveMerge functions used &dest->at(k_v.key()) which threw the exception. Remove RecursiveMerge completely and use the one implemented in jsoncons lib.
* add test
* replace RecursiveMerge with mergepatch::apply_merge_patch
* add exception handling for that flow
---------
Signed-off-by: kostas <kostas@dragonflydb.io>
The recent changes of the serialization_max_chunk_size set to 1 for extreme testing increased the running time of the tests causing them sometimes to timeout.
* increase timeout on reg tests from 40 to 50
---------
Signed-off-by: kostas <kostas@dragonflydb.io>
DastTable::Traverse is error prone when the callback passed preempts because the segment might change. This is problematic and we need atomicity while traversing segments with preemption. The fix is to add Traverse in DbSlice and protect the traversal via ThreadLocalMutex.
* add ConditionFlag to DbSlice
* add Traverse in DbSlice and protect it with the ConditionFlag
* remove condition flag from snapshot
* remove condition flag from streamer
---------
Signed-off-by: kostas <kostas@dragonflydb.io>
* feat: stabilize non-coordinated omission mode
1. Our latency/RPS computations were off because we started measuring before drivers
started running. Now, Run/Start phases are separated, so the start time is measured more precisely
(after the start phase)
2. Introduced progress per connection - one of my discoveries is that driver
connections progress with differrent pace when running in coordinated omission mode.
This can reach x5 speed differrences. Now we measure and output fastest/slowest progress.
3. Coordinated omission is great when the Server Under Test is able to sustain the required RPS.
But if the actual RPS is lower than the one is sent than the final latencies will be infinitely high.
We fix it by introducing self-adjusting sleep interval, so if the actual RPS is lower
we will increase the interval to be closer to the actual RPS.
Show p99 latency and maximum pending requests per connection.
Co-authored-by: Shahar Mike <chakaz@users.noreply.github.com>
Signed-off-by: Roman Gershman <romange@gmail.com>
---------
Signed-off-by: Roman Gershman <roman@dragonflydb.io>
Signed-off-by: Roman Gershman <romange@gmail.com>
Co-authored-by: Shahar Mike <chakaz@users.noreply.github.com>
* chore: Support setting the value of `replica-priority`
This PR adds a small refactor to the way we set and get config names
which have dashes (`-`) and underscores (`_`).
Until now, words were separated by underscores because this is how our
flags library (absl) works. However, this is incompatible with Valkey,
which uses dashes as a word separator.
Once merged, we will support both underscores and dashes in config
names, but will only return the name with dashes. **This is a behavior
change**.
We're doing this in order to be compatible with `replica-priority` and
possibly other config names that Valkey uses.
* Flag restore
* normalize to '_'
* fix: reenable macos builds
Also, add debug function that prints local state if deadlocks occure.
* fix: free cold memory for non-cache mode as well
* chore: disable FreeMemWithEvictionStep again
Because it heavily affects the performance when performing evictions.
---------
Signed-off-by: Roman Gershman <roman@dragonflydb.io>
1. Fully support tiered_experimental_cooling for all operations
2. Offset cool storage usage when computing memory pressure situations in Hearbeat.
3. Introduce realtime entry counting per db_slice and provide DCHECK to verify it vs the old approach.
Later we will switch to realtime entry and free memory computations when computing bytes per object,
and remove the old approach in CacheStats().
4. Show hit rate during the run of dfly_bench loadtest.
Signed-off-by: Roman Gershman <roman@dragonflydb.io>
