chore(replication-tests): add cache_mode on test replication all #2685

kostasrim · 2024-03-04T15:09:52Z

add cache_mode cases on test_replication_all
fix CVCOnBumpUp to not skip some of the modified buckets

The reg tests added uncovered a corner case with CVCOnBump. Basically, two buckets with version less than the threshold got modified but only one id got returned causing inconsistent data between the replica and master. This was fixed by properly returning the number of modified buckets to the caller.

kostasrim · 2024-03-04T15:11:59Z

tests/dragonfly/replication_test.py

+    async def check():
+        await check_all_replicas_finished(c_replicas, c_master)
+        hashes = await asyncio.gather(*(SeederV2.capture(c) for c in [c_master] + c_replicas))
+        assert len(set(hashes)) == 1


This is amazing 💯 (thank you @dranikpg 🙏 )

@adiholden We have an issue, this is flaky with cache mode and fails from time to time which means that some replica is not on par with master when we evict (cache mode on). I will investigate.

also (3). check could be a general function` this is a nice idiom :)

Thanks 😊

Hmmm.. shouldn't be the case, because we wait for replication lag to catch up

For (2) I suspect we have a bug in cache_mode

dranikpg · 2024-03-04T16:12:45Z

tests/dragonfly/replication_test.py

+    async def check():
+        await check_all_replicas_finished(c_replicas, c_master)
+        hashes = await asyncio.gather(*(SeederV2.capture(c) for c in [c_master] + c_replicas))
+        assert len(set(hashes)) == 1


Thanks 😊

Hmmm.. shouldn't be the case, because we wait for replication lag to catch up

tests/dragonfly/replication_test.py

kostasrim · 2024-03-07T11:36:50Z

@dranikpg it passes locally as well but it requires to increase the async_timeout from 20 seconds to 80 seconds (fails on 60seconds timeout). The question is, is this acceptable ? We are looking at 4x slower replication on cache mode for the stress test (pytest.param(8, [8, 8], dict(key_target=1_000_000, units=16), 50_000, marks=M_STRESS). cc @adiholden

dranikpg · 2024-03-07T12:10:43Z

We can skip the stress test, we can divide the workload by 2-3-4 if we see that we're running in cache mode, let you decide 🤷🏻

kostasrim · 2024-03-20T17:53:01Z

src/core/dash_internal.h

@@ -1506,46 +1506,22 @@ template <bool UV>
 std::enable_if_t<UV, unsigned> Segment<Key, Value, Policy>::CVCOnBump(uint64_t ver_threshold,
                                                                      unsigned bid, unsigned slot,
                                                                      Hash_t hash,
-                                                                      uint8_t result_bid[2]) const {
+                                                                      uint8_t result_bid[3]) const {


The problem here now is that we might send more buckets than we actually changed because there exist fast paths in Segment::BumpUp. For example, if we got empty space for some reason we will just unload the stash entry leaving the rest of the buckets untouched. As this was broken before, this is a small price for now but we should improve. I am still not 100% familiar with the dash code but I will investigate how we can patch this and make it better

src/core/dash_internal.h

romange · 2024-03-24T15:27:04Z

src/core/dash_internal.h

+  // 2. If there is empty space in target or probe bucket insert the slot there and remove
+  //    it from the stash bucket.
+  // 3. If there is no empty space then we need swap slots with either the target or the probe
+  //    bucket. Furthermore, we might clear the stash bucket so in total all the 3 buckets


Looks good!!!
Furthermore, we might clear the stash bucket - why we might clear the stash bucket?

Because we call target.UnsetStashPtr which internally calls ClearStash

ok, ClearStash clears a single entry inside a stash bucket, so maybe we might remove an entry from the stash bucket, so...

well, technically we do not remove an entry(slot) via ClearStash but rather clear the stash pointer of the bucket we are about to swap (since insertions after displacement end up on the stash buckets). I slightly reworded.

romange · 2024-03-24T15:27:59Z

src/core/dash_test.cc

+  // and both of them have version < version_threshold. If we don't return them both
+  // then full sync phase during replication might fail to capture the changes leading
+  // to data inconsistencies between master and replica.
+  EXPECT_EQ(touched_bid[1], 0);


romange · 2024-03-24T15:30:19Z

@kostasrim is this pr ready ?

kostasrim · 2024-03-26T08:52:33Z

@romange for some reason I received 0 notifications for this PR. Yes it's ready!

…nfly ( v1.15.1 → v1.16.0 ) (#3354) This PR contains the following updates: | Package | Update | Change | |---|---|---| | [docker.dragonflydb.io/dragonflydb/dragonfly](https://togithub.com/dragonflydb/dragonfly) | minor | `v1.15.1` -> `v1.16.0` | --- ### Release Notes <details> <summary>dragonflydb/dragonfly (docker.dragonflydb.io/dragonflydb/dragonfly)</summary> ### [`v1.16.0`](https://togithub.com/dragonflydb/dragonfly/releases/tag/v1.16.0) [Compare Source](https://togithub.com/dragonflydb/dragonfly/compare/v1.15.1...v1.16.0) ##### Dragonfly v1.16.0 Our spring release. We are getting closer to 2.0 with some very exciting features ahead. Stay tuned! Some prominent changes include: - Improved memory accounting of client connections ([#2710](https://togithub.com/dragonflydb/dragonfly/issues/2710) [#2755](https://togithub.com/dragonflydb/dragonfly/issues/2755) and [#2692](https://togithub.com/dragonflydb/dragonfly/issues/2692) ) - FT.AGGREGATE call ([#2413](https://togithub.com/dragonflydb/dragonfly/issues/2413)) - Properly handle and replicate Memcache flags ([#2787](https://togithub.com/dragonflydb/dragonfly/issues/2787) [#2807](https://togithub.com/dragonflydb/dragonfly/issues/2807)) - Intoduce BF.AGGREGATE BD.(M)ADD and BF.(M)EXISTS methods ([#2801](https://togithub.com/dragonflydb/dragonfly/issues/2801)). Note, that it does not work with snapshots and replication yet. - Dragonfly builds natively on MacOS. We would love some help with extending the release pipeline to create a proper macos binary. - Following the requests from the Edge developers community, we added a basic HTTP API support! Try running Dragonfly with: `--expose_http_api` flag and then call `curl -X POST -d '["ping"]' localhost:6379/api`. We will follow up with more extensive docs later this month. - Lots of stability fixes, especially around Sidekiq and BullMQ workloads. ##### What's Changed - chore: make usan asan optional and enable them on CI by [@kostasrim](https://togithub.com/kostasrim) in [dragonflydb/dragonfly#2631 - fix: missing manual trigger for daily sanitizers by [@kostasrim](https://togithub.com/kostasrim) in [dragonflydb/dragonfly#2682 - bug(tiering): fix overflow in page offset calculation and wrong hash offset calculation by [@theyueli](https://togithub.com/theyueli) in [dragonflydb/dragonfly#2683 - Chore: Fixed Docker Health Check by [@manojks1999](https://togithub.com/manojks1999) in [dragonflydb/dragonfly#2659 - chore: Increase disk space in the coverage runs by [@romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2684 - fix(flushall): Decommit memory after releasing tables. by [@chakaz](https://togithub.com/chakaz) in [dragonflydb/dragonfly#2691 - feat(server): Account for serializer's temporary buffer size by [@chakaz](https://togithub.com/chakaz) in [dragonflydb/dragonfly#2689 - refactor: remove FULL-SYNC-CUT cmd [#2687](https://togithub.com/dragonflydb/dragonfly/issues/2687) by [@BorysTheDev](https://togithub.com/BorysTheDev) in [dragonflydb/dragonfly#2688 - chore: add malloc-based stats and decommit by [@romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2692 - feat(cluster): automatic slot migration finalization [#2697](https://togithub.com/dragonflydb/dragonfly/issues/2697) by [@BorysTheDev](https://togithub.com/BorysTheDev) in [dragonflydb/dragonfly#2698 - Basic FT.AGGREGATE by [@dranikpg](https://togithub.com/dranikpg) in [dragonflydb/dragonfly#2413 - chore: little transaction cleanup by [@dranikpg](https://togithub.com/dranikpg) in [dragonflydb/dragonfly#2608 - fix(channel store): add acquire/release pair in fast update path by [@dranikpg](https://togithub.com/dranikpg) in [dragonflydb/dragonfly#2704 - chore: add ubuntu22 devcontainer by [@romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2700 - feat(cluster): Add `--cluster_id` flag by [@chakaz](https://togithub.com/chakaz) in [dragonflydb/dragonfly#2695 - feat(server): Use mimalloc in SSL calls by [@chakaz](https://togithub.com/chakaz) in [dragonflydb/dragonfly#2710 - chore: Pull helio with new BlockingCounter by [@dranikpg](https://togithub.com/dranikpg) in [dragonflydb/dragonfly#2711 - chore(transaction): Simplify PollExecution by [@dranikpg](https://togithub.com/dranikpg) in [dragonflydb/dragonfly#2712 - chore(transaction): Don't call GetLocalMask from blocking controller by [@dranikpg](https://togithub.com/dranikpg) in [dragonflydb/dragonfly#2715 - chore: improve compatibility of EXPIRE functions with Redis by [@romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2696 - chore: disable flaky fuzzy migration test by [@kostasrim](https://togithub.com/kostasrim) in [dragonflydb/dragonfly#2716 - chore: update sanitizers workflow by [@kostasrim](https://togithub.com/kostasrim) in [dragonflydb/dragonfly#2686 - chore: Use c-ares for resolving hosts in `ProtocolClient` by [@chakaz](https://togithub.com/chakaz) in [dragonflydb/dragonfly#2719 - Remove check-fail in ExpireIfNeeded and introduce DFLY LOAD by [@romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2699 - chore: Record cmd stat from invoke by [@dranikpg](https://togithub.com/dranikpg) in [dragonflydb/dragonfly#2720 - fix(transaction): nullptr access on non transactional commands by [@kostasrim](https://togithub.com/kostasrim) in [dragonflydb/dragonfly#2724 - fix(BgSave): async from sync by [@kostasrim](https://togithub.com/kostasrim) in [dragonflydb/dragonfly#2702 - chore: remove core/fibers by [@romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2723 - fix(transaction): Replace with armed sync point by [@dranikpg](https://togithub.com/dranikpg) in [dragonflydb/dragonfly#2708 - feat(json): Deserialize ReJSON format by [@chakaz](https://togithub.com/chakaz) in [dragonflydb/dragonfly#2725 - feat: add flag masteruser by [@kostasrim](https://togithub.com/kostasrim) in [dragonflydb/dragonfly#2693 - refactor: block list refactoring [#2580](https://togithub.com/dragonflydb/dragonfly/issues/2580) by [@BorysTheDev](https://togithub.com/BorysTheDev) in [dragonflydb/dragonfly#2732 - chore: fix DeduceExecMode by [@dranikpg](https://togithub.com/dranikpg) in [dragonflydb/dragonfly#2733 - fix(cluster): Reply with correct `\n` / `\r\n` from `CLUSTER` sub cmd by [@chakaz](https://togithub.com/chakaz) in [dragonflydb/dragonfly#2731 - chore: Introduce fiber stack allocator by [@romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2730 - fix(cluster): Save replica ID per replica by [@chakaz](https://togithub.com/chakaz) in [dragonflydb/dragonfly#2735 - fix(ssl): Proper cleanup by [@chakaz](https://togithub.com/chakaz) in [dragonflydb/dragonfly#2742 - chore: add skeleton files for flat_dfs code by [@romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2738 - chore: better error reporting when connecting to tls with plain socket by [@romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2740 - chore: Support json paths without root selector by [@dranikpg](https://togithub.com/dranikpg) in [dragonflydb/dragonfly#2747 - chore: journal cleanup by [@romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2749 - refactor: remove start-slot-migration cmd [#2727](https://togithub.com/dragonflydb/dragonfly/issues/2727) by [@BorysTheDev](https://togithub.com/BorysTheDev) in [dragonflydb/dragonfly#2728 - feat(server): Add TLS usage to /metrics and `INFO MEMORY` by [@chakaz](https://togithub.com/chakaz) in [dragonflydb/dragonfly#2755 - chore: preparations for adding flat json support by [@romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2752 - chore(transaction): Introduce RunCallback by [@dranikpg](https://togithub.com/dranikpg) in [dragonflydb/dragonfly#2760 - feat(replication): Do not auto replicate different master by [@chakaz](https://togithub.com/chakaz) in [dragonflydb/dragonfly#2753 - Improve Helm chart to be rendered locally and on machines where is not the application target by [@fafg](https://togithub.com/fafg) in [dragonflydb/dragonfly#2706 - chore: preparation for basic http api by [@romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2764 - feat(server): Add metric for RDB save duration. by [@chakaz](https://togithub.com/chakaz) in [dragonflydb/dragonfly#2768 - chore: fix flat_dfs read tests. by [@romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2772 - fix(ci): do not overwrite last_log_file among tests by [@kostasrim](https://togithub.com/kostasrim) in [dragonflydb/dragonfly#2759 - feat(server): support cluster replication by [@adiholden](https://togithub.com/adiholden) in [dragonflydb/dragonfly#2748 - fix: fiber preempts on read path and OnCbFinish() clears fetched_items\_ by [@kostasrim](https://togithub.com/kostasrim) in [dragonflydb/dragonfly#2763 - chore(ci): open last_log_file in append mode by [@kostasrim](https://togithub.com/kostasrim) in [dragonflydb/dragonfly#2776 - doc(README): fix outdated expiry ranges description by [@enjoy-binbin](https://togithub.com/enjoy-binbin) in [dragonflydb/dragonfly#2779 - Benchmark runner by [@adiholden](https://togithub.com/adiholden) in [dragonflydb/dragonfly#2780 - chore(replication-tests): add cache_mode on test replication all by [@kostasrim](https://togithub.com/kostasrim) in [dragonflydb/dragonfly#2685 - feat(tiering): DiskStorage by [@dranikpg](https://togithub.com/dranikpg) in [dragonflydb/dragonfly#2770 - chore: add a boilerplate for bloom filter family by [@romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2782 - chore: introduce conversion routines between JsonType and FlatJson by [@romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2785 - chore: Fix memcached flags not updated by [@dranikpg](https://togithub.com/dranikpg) in [dragonflydb/dragonfly#2787 - chore: remove duplicate code from dash and simplify by [@kostasrim](https://togithub.com/kostasrim) in [dragonflydb/dragonfly#2765 - chore: disable test_cluster_slot_migration by [@kostasrim](https://togithub.com/kostasrim) in [dragonflydb/dragonfly#2788 - fix: new\[] delete\[] missmatch in disk_storage by [@kostasrim](https://togithub.com/kostasrim) in [dragonflydb/dragonfly#2792 - fix: sanitizers clang build and clean up some warnings by [@kostasrim](https://togithub.com/kostasrim) in [dragonflydb/dragonfly#2793 - chore: add bloom filter class by [@romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2791 - chore: add SBF data structure by [@romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2795 - chore(tiering): Disable compilation for MacOs by [@dranikpg](https://togithub.com/dranikpg) in [dragonflydb/dragonfly#2799 - chore: fix daily build by [@romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2798 - chore: expose SBF via compact_object by [@romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2797 - fix(ci): malloc trim on sanitizers workflow by [@kostasrim](https://togithub.com/kostasrim) in [dragonflydb/dragonfly#2794 - fix(cluster): Don't miss updates in FLUSHSLOTS by [@chakaz](https://togithub.com/chakaz) in [dragonflydb/dragonfly#2783 - feat: add bf.(m)add and bf.(m)exists commands by [@romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2801 - fix: SBF memory leaks by [@kostasrim](https://togithub.com/kostasrim) in [dragonflydb/dragonfly#2803 - chore: refactor StringFamily::Set to use CmdArgParser by [@romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2800 - fix: propagate memcached flags to replica by [@romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2807 - DFLYMIGRATE ACK refactoring by [@BorysTheDev](https://togithub.com/BorysTheDev) in [dragonflydb/dragonfly#2790 - feat: add master lsn and journal_executed dcheck in replica via ping by [@kostasrim](https://togithub.com/kostasrim) in [dragonflydb/dragonfly#2778 - fix: correct json response for errors by [@romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2813 - chore: bloom test - cover corner cases by [@romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2806 - bug(server): do not write lsn opcode to journal by [@adiholden](https://togithub.com/adiholden) in [dragonflydb/dragonfly#2814 - chore: Fix build by disabling the tests. by [@romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2821 - fix(replication): replication with multi shard sync enabled lagging by [@adiholden](https://togithub.com/adiholden) in [dragonflydb/dragonfly#2823 - fix: io_uring/fibers bug in DnsResolve by [@romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2825 ##### New Contributors - [@manojks1999](https://togithub.com/manojks1999) made their first contribution in [dragonflydb/dragonfly#2659 - [@fafg](https://togithub.com/fafg) made their first contribution in [dragonflydb/dragonfly#2706 - [@enjoy-binbin](https://togithub.com/enjoy-binbin) made their first contribution in [dragonflydb/dragonfly#2779 ##### Huge thanks to all the contributors! ❤️ 🇮🇱 🇺🇦 **Full Changelog**: dragonflydb/dragonfly@v1.15.0...v1.16.0 </details>  Co-authored-by: repo-jeeves[bot] <106431701+repo-jeeves[bot]@users.noreply.github.com>

chore(replication-tests): add cache_mode on test replication all

4ad21bb

kostasrim self-assigned this Mar 4, 2024

kostasrim commented Mar 4, 2024

View reviewed changes

kostasrim requested a review from adiholden March 4, 2024 15:12

dranikpg previously approved these changes Mar 4, 2024

View reviewed changes

dranikpg reviewed Mar 4, 2024

View reviewed changes

tests/dragonfly/replication_test.py Show resolved Hide resolved

add missing await

dbb42f5

kostasrim dismissed dranikpg’s stale review via dbb42f5 March 7, 2024 11:38

kostasrim added 2 commits March 11, 2024 11:02

skip stress test

eb047b1

Merge branch 'main' into add_cache_mode_to_replication_test_all

4c4ca02

kostasrim requested a review from dranikpg March 11, 2024 09:03

kostasrim added 2 commits March 20, 2024 19:38

fix CVCOnBumpUp

fb1544b

small fix

af43757

kostasrim commented Mar 20, 2024

View reviewed changes

romange reviewed Mar 21, 2024

View reviewed changes

src/core/dash_internal.h Show resolved Hide resolved

address comments

759e6b7

kostasrim requested a review from romange March 21, 2024 15:18

romange reviewed Mar 24, 2024

View reviewed changes

kostasrim requested a review from romange March 26, 2024 08:52

romange removed their request for review March 26, 2024 09:08

reword comments

d54ae37

kostasrim requested a review from romange March 26, 2024 14:33

Merge branch 'main' into add_cache_mode_to_replication_test_all

f79c401

romange approved these changes Mar 27, 2024

View reviewed changes

kostasrim merged commit cd20c40 into main Mar 27, 2024
10 checks passed

kostasrim deleted the add_cache_mode_to_replication_test_all branch March 27, 2024 12:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(replication-tests): add cache_mode on test replication all #2685

chore(replication-tests): add cache_mode on test replication all #2685

kostasrim commented Mar 4, 2024 •

edited

kostasrim Mar 4, 2024 •

edited

kostasrim Mar 4, 2024

dranikpg Mar 4, 2024

kostasrim Mar 4, 2024

dranikpg Mar 4, 2024

kostasrim commented Mar 7, 2024

dranikpg commented Mar 7, 2024

kostasrim Mar 20, 2024

romange Mar 24, 2024

kostasrim Mar 26, 2024

romange Mar 26, 2024

kostasrim Mar 26, 2024 •

edited

romange Mar 24, 2024

romange commented Mar 24, 2024

kostasrim commented Mar 26, 2024

chore(replication-tests): add cache_mode on test replication all #2685

chore(replication-tests): add cache_mode on test replication all #2685

Conversation

kostasrim commented Mar 4, 2024 • edited

kostasrim Mar 4, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kostasrim commented Mar 7, 2024

dranikpg commented Mar 7, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kostasrim Mar 26, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

romange commented Mar 24, 2024

kostasrim commented Mar 26, 2024

kostasrim commented Mar 4, 2024 •

edited

kostasrim Mar 4, 2024 •

edited

kostasrim Mar 26, 2024 •

edited