Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: JSON.MSET command #3459

Merged
merged 1 commit into from
Aug 6, 2024
Merged

fix: JSON.MSET command #3459

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/server/json_family.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2233,7 +2233,8 @@ void JsonFamily::Get(CmdArgList args, ConnectionContext* cntx) {
// TODO: Add sensible defaults/categories to json commands

void JsonFamily::Register(CommandRegistry* registry) {
constexpr size_t kMsetFlags = CO::WRITE | CO::DENYOOM | CO::FAST | CO::INTERLEAVED_KEYS;
constexpr size_t kMsetFlags =
CO::WRITE | CO::DENYOOM | CO::FAST | CO::INTERLEAVED_KEYS | CO::NO_AUTOJOURNAL;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I am missing something but from transaction.cc

  1363   // If autojournaling was disabled and not re-enabled the callback is writing to journal.
  1364   // We do not allow preemption in callbacks and therefor the call to RecordJournal from
  1365   // from callbacks does not allow await.
  1366   // To make sure we flush the changes to sync we call TriggerJournalWriteToSink here.
  1367   if ((cid_->opt_mask() & CO::NO_AUTOJOURNAL) && !re_enabled_auto_journal_) {
  1368     TriggerJournalWriteToSink();
  1369     return; 
  1370   }

So when CO::NO_AUTOJOURNAL is added then it means we also write to the journal the opposite of the claim that we write it twice no ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't get what do you want to say, but the code that you have added is journal->RecordEntry(0, journal::Op::NOOP, 0, 0, nullopt, {}, true);

registry->StartFamily();
*registry << CI{"JSON.GET", CO::READONLY | CO::FAST, -2, 1, 1, acl::JSON}.HFUNC(Get);
*registry << CI{"JSON.MGET", CO::READONLY | CO::FAST, -3, 1, -2, acl::JSON}.HFUNC(MGet);
Expand Down
4 changes: 1 addition & 3 deletions tests/dragonfly/replication_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1972,7 +1972,6 @@ async def test_heartbeat_eviction_propagation(df_factory):
await disconnect_clients(c_master, *[c_replica])


@pytest.mark.skip(reason="Test is flaky")
@pytest.mark.asyncio
async def test_policy_based_eviction_propagation(df_factory, df_seeder_factory):
master = df_factory.create(
Expand Down Expand Up @@ -2005,8 +2004,7 @@ async def test_policy_based_eviction_propagation(df_factory, df_seeder_factory):
keys_master = await c_master.execute_command("keys k*")
keys_replica = await c_replica.execute_command("keys k*")

assert len(keys_master) == len(keys_replica)
assert set(keys_master) == set(keys_replica)
assert set(keys_replica).difference(keys_master) == set()

await disconnect_clients(c_master, *[c_replica])

Expand Down
Loading