Skip to content

Commit 8dd9e55

Browse files
BagritsevichStepanromange
authored andcommitted
chore: Limit expiry value to 32 bits.
We use adaptive precision, where we keep "millis" precision if we can, and switch to seconds precision if the deadline is too large. This actually was before but now I reduced the cut-off to 4B ms, or 49 days. Also, the ttl in seconds is now limited to 4B sec, with larger values quietly rounded down to this limit. Signed-off-by: Roman Gershman <[email protected]>
1 parent 2d0dd3c commit 8dd9e55

File tree

5 files changed

+39
-20
lines changed

5 files changed

+39
-20
lines changed

src/core/expire_period.h

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,13 @@
88

99
namespace dfly {
1010

11+
// ExpirePeriod encapsulates the expiration period of a key.
12+
// It can represent periods in milliseconds up to ~49 days and in seconds up to ~136 years.
13+
// If value in milliseconds is too high, it switches to seconds precision.
14+
// And if it's still too high, it silently clamps to the max value.
1115
class ExpirePeriod {
1216
public:
13-
static constexpr size_t kMaxGenId = 15;
17+
static constexpr uint32_t kMaxGenId = 15;
1418

1519
ExpirePeriod() : val_(0), gen_(0), precision_(0) {
1620
static_assert(sizeof(ExpirePeriod) == 8); // TODO
@@ -35,28 +39,30 @@ class ExpirePeriod {
3539

3640
void Set(uint64_t ms);
3741

38-
bool is_second_precision() { return precision_ == 1;}
42+
bool is_second_precision() const {
43+
return precision_ == 1;
44+
}
3945

4046
private:
41-
uint64_t val_ : 59;
42-
uint64_t gen_ : 4;
43-
uint64_t precision_ : 1; // 0 - ms, 1 - sec.
47+
uint32_t val_;
48+
uint8_t gen_ : 2; // generation id.
49+
uint8_t precision_ : 1; // 0 - ms, 1 - sec.
4450
};
4551

4652
inline void ExpirePeriod::Set(uint64_t ms) {
47-
constexpr uint64_t kBarrier = (1ULL << 48);
48-
49-
if (ms < kBarrier) {
50-
val_ = ms;
51-
precision_ = 0; // ms
53+
if (ms < UINT32_MAX) {
54+
val_ = ms; // about 49 days in ms.
55+
precision_ = 0; // ms
5256
return;
5357
}
5458

55-
precision_ = 1;
56-
if (ms < kBarrier << 10) {
57-
ms = (ms + 500) / 1000; // seconds
59+
precision_ = 1; // seconds
60+
if (ms < UINT64_MAX / 2) {
61+
ms = (ms + 500) / 1000;
62+
val_ = ms >= UINT32_MAX ? UINT32_MAX - 1 : ms;
63+
} else {
64+
val_ = UINT32_MAX - 1;
5865
}
59-
val_ = ms >= kBarrier ? kBarrier - 1 : ms;
6066
}
6167

6268
} // namespace dfly

src/server/db_slice.cc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -398,6 +398,8 @@ class DbSlice::PrimeBumpPolicy {
398398
DbSlice::DbSlice(uint32_t index, bool cache_mode, EngineShard* owner)
399399
: shard_id_(index),
400400
cache_mode_(cache_mode),
401+
expire_allowed_(true),
402+
expire_base_index_(0),
401403
owner_(owner),
402404
client_tracking_map_(owner->memory_resource()) {
403405
db_arr_.emplace_back();

src/server/db_slice.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -252,11 +252,11 @@ class DbSlice {
252252
}
253253

254254
int64_t ExpireTime(const ExpirePeriod& val) const {
255-
return expire_base_[0] + val.duration_ms();
255+
return expire_base_[val.generation_id()] + val.duration_ms();
256256
}
257257

258258
ExpirePeriod FromAbsoluteTime(uint64_t time_ms) const {
259-
return ExpirePeriod{time_ms - expire_base_[0]};
259+
return ExpirePeriod{time_ms - expire_base_[expire_base_index_], expire_base_index_};
260260
}
261261

262262
struct ItAndUpdater {
@@ -607,11 +607,11 @@ class DbSlice {
607607

608608
ShardId shard_id_;
609609
uint8_t cache_mode_ : 1;
610-
610+
uint8_t expire_allowed_ : 1;
611+
uint8_t expire_base_index_ : 1;
611612
EngineShard* owner_;
612613

613614
int64_t expire_base_[2]; // Used for expire logic, represents a real clock.
614-
bool expire_allowed_ = true;
615615

616616
uint64_t version_ = 1; // Used to version entries in the PrimeTable.
617617
uint64_t next_moved_id_ = 1;

src/server/engine_shard.cc

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,8 @@ std::optional<CollectedPageStats> EngineShard::DoDefrag(CollectPageStats collect
301301
return page_usage.CollectedStats();
302302
}
303303

304+
constexpr uint32_t kRunAtLowPriority = 0u;
305+
304306
// the memory defragmentation task is as follow:
305307
// 1. Check if memory usage is high enough
306308
// 2. Check if diff between commited and used memory is high enough
@@ -310,7 +312,6 @@ std::optional<CollectedPageStats> EngineShard::DoDefrag(CollectPageStats collect
310312
// priority.
311313
// otherwise lower the task priority so that it would not use the CPU when not required
312314
uint32_t EngineShard::DefragTask() {
313-
constexpr uint32_t kRunAtLowPriority = 0u;
314315
if (!namespaces) {
315316
return kRunAtLowPriority;
316317
}
@@ -326,6 +327,10 @@ uint32_t EngineShard::DefragTask() {
326327
return 6; // priority.
327328
}
328329

330+
uint32_t EngineShard::UpdateExpiresTask() {
331+
return kRunAtLowPriority;
332+
}
333+
329334
EngineShard::EngineShard(util::ProactorBase* pb, mi_heap_t* heap)
330335
: txq_([](const Transaction* t) { return t->txid(); }),
331336
queue_(kQueueLen, 1, 1),
@@ -347,6 +352,8 @@ void EngineShard::Shutdown() {
347352

348353
void EngineShard::StopPeriodicFiber() {
349354
ProactorBase::me()->RemoveOnIdleTask(defrag_task_);
355+
ProactorBase::me()->RemoveOnIdleTask(update_expire_base_task_);
356+
350357
fiber_heartbeat_periodic_done_.Notify();
351358
if (fiber_heartbeat_periodic_.IsJoinable()) {
352359
fiber_heartbeat_periodic_.Join();
@@ -394,6 +401,7 @@ void EngineShard::StartPeriodicHeartbeatFiber(util::ProactorBase* pb) {
394401
RunFPeriodically(heartbeat, period_ms, "heartbeat", &fiber_heartbeat_periodic_done_);
395402
});
396403
defrag_task_ = pb->AddOnIdleTask([this]() { return DefragTask(); });
404+
update_expire_base_task_ = pb->AddOnIdleTask([this]() { return UpdateExpiresTask(); });
397405
}
398406

399407
void EngineShard::StartPeriodicShardHandlerFiber(util::ProactorBase* pb,

src/server/engine_shard.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,7 @@ class EngineShard {
257257
// context of the controlling thread will access this shard!
258258
// --------------------------------------------------------------------------
259259
uint32_t DefragTask();
260+
uint32_t UpdateExpiresTask();
260261

261262
TxQueue txq_;
262263
TaskQueue queue_, queue2_;
@@ -283,7 +284,9 @@ class EngineShard {
283284
journal::Journal* journal_ = nullptr;
284285
IntentLock shard_lock_;
285286

286-
uint32_t defrag_task_ = 0;
287+
// Idle tasks.
288+
uint32_t defrag_task_ = 0, update_expire_base_task_ = 0;
289+
287290
EvictionTaskState eviction_state_; // Used on eviction fiber
288291
util::fb2::Fiber fiber_heartbeat_periodic_;
289292
util::fb2::Done fiber_heartbeat_periodic_done_;

0 commit comments

Comments
 (0)