Skip to content

Commit 370ef9c

Browse files
authored
chore: reduce frequency of clock invocations (#5946)
Partially addresses #5942 as now squashing transactions read clock only once. Moreover for most transactions we called clock twice - in the c'tor and during scheduling. This change removes the scheduling override unless transaction is repeated. Signed-off-by: Roman Gershman <[email protected]>
1 parent cf4d08d commit 370ef9c

File tree

5 files changed

+17
-8
lines changed

5 files changed

+17
-8
lines changed

src/server/generic_family.cc

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -960,7 +960,10 @@ OpResult<uint64_t> OpTtl(Transaction* t, EngineShard* shard, string_view key) {
960960
auto opExpireTimeResult = OpExpireTime(t, shard, key);
961961

962962
if (opExpireTimeResult) {
963-
int64_t ttl_ms = opExpireTimeResult.value() - t->GetDbContext().time_now_ms;
963+
auto now = t->GetDbContext().time_now_ms;
964+
DCHECK_GT(now, 0u);
965+
966+
int64_t ttl_ms = opExpireTimeResult.value() - now;
964967
DCHECK_GT(ttl_ms, 0); // Otherwise FindReadOnly would return null.
965968
return ttl_ms;
966969
} else {
@@ -1898,6 +1901,7 @@ void GenericFamily::Time(CmdArgList args, const CommandContext& cmd_cntx) {
18981901
} else {
18991902
now_usec = absl::GetCurrentTimeNanos() / 1000;
19001903
}
1904+
DCHECK_GT(now_usec, 0u);
19011905

19021906
auto* rb = static_cast<RedisReplyBuilder*>(cmd_cntx.rb);
19031907
rb->StartArray(2);

src/server/server_family.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3886,7 +3886,7 @@ void ServerFamily::Role(CmdArgList args, const CommandContext& cmd_cntx) {
38863886
rb->StartArray(4 + cluster_replicas_.size() * 3);
38873887
rb->SendBulkString(GetFlag(FLAGS_info_replication_valkey_compatible) ? "slave" : "replica");
38883888

3889-
auto send_replica_info = [rb](Replica::Summary rinfo) {
3889+
auto send_replica_info = [rb](const Replica::Summary& rinfo) {
38903890
rb->SendBulkString(rinfo.host);
38913891
rb->SendBulkString(absl::StrCat(rinfo.port));
38923892
if (rinfo.full_sync_done) {
@@ -3908,7 +3908,7 @@ void ServerFamily::Role(CmdArgList args, const CommandContext& cmd_cntx) {
39083908
}
39093909

39103910
void ServerFamily::Script(CmdArgList args, const CommandContext& cmd_cntx) {
3911-
script_mgr_->Run(std::move(args), cmd_cntx.tx, cmd_cntx.rb, cmd_cntx.conn_cntx);
3911+
script_mgr_->Run(args, cmd_cntx.tx, cmd_cntx.rb, cmd_cntx.conn_cntx);
39123912
}
39133913

39143914
void ServerFamily::LastSave(CmdArgList args, const CommandContext& cmd_cntx) {

src/server/stream_family.cc

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2808,8 +2808,10 @@ void StreamFamily::XClaim(CmdArgList args, const CommandContext& cmd_cntx) {
28082808
if (!ParseXclaimOptions(args, opts, cmd_cntx.rb))
28092809
return;
28102810

2811-
if (uint64_t now = cmd_cntx.tx->GetDbContext().time_now_ms;
2812-
opts.delivery_time < 0 || static_cast<uint64_t>(opts.delivery_time) > now)
2811+
uint64_t now = cmd_cntx.tx->GetDbContext().time_now_ms;
2812+
DCHECK_GT(now, 0u);
2813+
2814+
if (opts.delivery_time < 0 || static_cast<uint64_t>(opts.delivery_time) > now)
28132815
opts.delivery_time = now;
28142816

28152817
auto cb = [&](Transaction* t, EngineShard* shard) {

src/server/stream_family_test.cc

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -599,9 +599,11 @@ TEST_F(StreamFamilyTest, Xclaim) {
599599
resp = Run({"xclaim", "foo", "group", "alice", "0", "1-4", "TIME",
600600
absl::StrCat(TEST_current_time_ms - 500), "justid"});
601601
EXPECT_THAT(resp.GetString(), "1-4");
602+
602603
// min idle time is exceeded for this entry
603604
resp = Run({"xclaim", "foo", "group", "bob", "600", "1-4"});
604-
EXPECT_THAT(resp, ArrLen(0));
605+
ASSERT_THAT(resp, ArrLen(0));
606+
605607
resp = Run({"xclaim", "foo", "group", "bob", "400", "1-4", "justid"});
606608
EXPECT_THAT(resp.GetString(), "1-4");
607609

src/server/transaction.cc

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -542,6 +542,8 @@ void Transaction::MultiUpdateWithParent(const Transaction* parent) {
542542
// DCHECK(multi_);
543543
// DCHECK(parent->multi_); // it might not be a squasher yet, but certainly is multi
544544
DCHECK_EQ(multi_->role, SQUASHED_STUB);
545+
DCHECK(parent->time_now_ms_);
546+
545547
txid_ = parent->txid_;
546548
time_now_ms_ = parent->time_now_ms_;
547549
unique_slot_checker_ = parent->unique_slot_checker_;
@@ -752,8 +754,6 @@ void Transaction::ScheduleInternal() {
752754
if (unique_shard_cnt_ > 1)
753755
txid_ = op_seq.fetch_add(1, memory_order_relaxed);
754756

755-
InitTxTime();
756-
757757
run_barrier_.Start(unique_shard_cnt_);
758758

759759
if (CanRunInlined()) {
@@ -828,6 +828,7 @@ void Transaction::ScheduleInternal() {
828828
i, [](unsigned) { EngineShard::tlocal()->PollExecution("cancel_cleanup", nullptr); });
829829
});
830830
}
831+
InitTxTime(); // update time for next scheduling attempt
831832
}
832833

833834
coordinator_state_ |= COORD_SCHED;

0 commit comments

Comments
 (0)