Skip to content

feat: add diagnostic Redis TTL bypass #546

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

Open
thweetkomputer wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: add diagnostic Redis TTL bypass #546

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
1 change: 1 addition & 0 deletions docs/09-store-handler.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ For `ELOQDSS_ROCKSDB_CLOUD_S3`, a `FileCacheSyncWorker` periodically sends the p
## 6. TTL and Purge Mechanisms

- **Record TTL (data plane).** `BatchWriteRecords` items carry a `ttl` (ms epoch). On read, `FetchRecordCallback` treats an expired EloqKV record as `RecordStatus::Deleted` even if the store still has it. Physical reclamation is compaction-driven: `TTLCompactionFilter` (both `rocksdb_handler.h` and DSS `rocksdb_data_store_common.h`) drops expired entries during RocksDB compaction; the DSS variant flags TTL presence in the version-ts MSB (`MSB`/`MSB_MASK`).
- **Diagnostic TTL bypass (EloqDSS RocksDB variants).** `--ignore_redis_ttl=true` (or `[store] ignore_redis_ttl=true`) bypasses the EloqKV point-read, bucket, and scanner expiry checks and makes `TTLCompactionFilter` retain expired records. EloqKV separately makes its TTL object variants report no TTL while the mode is active. Neither the version-ts MSB, the outer expiration timestamp, nor the Redis object's embedded TTL is rewritten, so restarting every EloqKV and DSS process with the option disabled restores normal expiration from the stored absolute timestamp. All participating processes must use the same setting; the option is intended for isolated diagnostic stores, and it does not make ordinary writes read-only.

@coderabbitai coderabbitai Bot Aug 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Document command-line precedence.

An explicit --ignore_redis_ttl value overrides [store] ignore_redis_ttl. State this rule, including that --ignore_redis_ttl=false overrides an INI value of true.

As per coding guidelines, document non-obvious operational constraints and explain why.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/09-store-handler.md` at line 127, Update the “Diagnostic TTL bypass
(EloqDSS RocksDB variants)” documentation to state that an explicitly supplied
--ignore_redis_ttl command-line value takes precedence over [store]
ignore_redis_ttl, including that --ignore_redis_ttl=false overrides an INI value
of true. Briefly explain the operational constraint that all participating
processes must use the same effective setting.

Source: Coding guidelines

- **Scan-iterator TTL (`TTLWrapperCache`, `data_store_service.h/.cpp`).** Open scan sessions cache their RocksDB iterator (`RocksDBIteratorTTLWrapper`) keyed by session id. A per-shard `dss_ttl` worker wakes every 3 s and erases not-in-use wrappers idle longer than the interval; `Borrow`/`Return` mark in-use; shard close force-erases.
- **Cloud SST purger (`purger_event_listener.h`, `purger_sliding_window.h`).** rocksdb-cloud's background purger deletes obsolete S3 files. `PurgerEventListener` tracks live file numbers across flush/compaction; a time-based `SlidingWindow` publishes the smallest in-use file number to S3 (`S3FileNumberUpdater`) so the purger never deletes a file a lagging reader (standby syncing the file cache) may still need, and can temporarily block the purger.

Expand Down
5 changes: 3 additions & 2 deletions store_handler/data_store_service_client_closure.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include <utility>

#include "cc_req_misc.h"
#include "eloq_data_store_service/ignore_redis_ttl.h"
#include "error_messages.h"
#include "store_util.h" // host_to_big_endian
#include "tx_service/include/cc/cc_request.h"
Expand Down Expand Up @@ -172,7 +173,7 @@ void FetchRecordCallback(void *data,
{
// Hash partition
const uint64_t rec_ttl = read_closure->Ttl();
if (rec_ttl > 0 &&
if (!IgnoreRedisTTL() && rec_ttl > 0 &&
rec_ttl < txservice::LocalCcShards::ClockTsInMillseconds())
{
// expired record
Expand Down Expand Up @@ -277,7 +278,7 @@ void FetchBucketDataCallback(void *data,
{
scan_next_closure->GetItem(item_idx, key_str, value_str, ts, ttl);
last_scanned_key = key_str;
if (ttl > 0 && ttl < now)
if (!IgnoreRedisTTL() && ttl > 0 && ttl < now)
{
// fetch_bucket_data_cc->AddDataItem(std::move(tx_key), "", 1,
// true);
Expand Down
3 changes: 2 additions & 1 deletion store_handler/data_store_service_scanner.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include <vector>

#include "data_store_service_client_closure.h"
#include "eloq_data_store_service/ignore_redis_ttl.h"
#include "eloq_data_store_service/object_pool.h"
#include "tx_service/include/tx_key.h"

Expand Down Expand Up @@ -154,7 +155,7 @@ void SinglePartitionScanner::ProcessScanNextResult(
sp_scanner->last_key_ = key;
}

if (ttl > 0 && ttl < now)
if (!IgnoreRedisTTL() && ttl > 0 && ttl < now)
{
// TTL expired record
DLOG(INFO) << "TTL expired record, key: " << key << ", ttl: " << ttl
Expand Down
38 changes: 38 additions & 0 deletions store_handler/eloq_data_store_service/ignore_redis_ttl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/**
* Copyright (C) 2026 EloqData Inc.
*
* This program is free software: you can redistribute it and/or modify
* it under either of the following two licenses:
* 1. GNU Affero General Public License, version 3, as published by the Free
* Software Foundation.
* 2. GNU General Public License as published by the Free Software
* Foundation; version 2 of the License.
*/

#pragma once

#include <gflags/gflags_declare.h>

#if defined(DATA_STORE_TYPE_ELOQDSS_ROCKSDB) || \
defined(DATA_STORE_TYPE_ELOQDSS_ROCKSDB_CLOUD_S3) || \
defined(DATA_STORE_TYPE_ELOQDSS_ROCKSDB_CLOUD_GCS)
DECLARE_bool(ignore_redis_ttl);
#endif

namespace EloqDS
{
/**
* Returns whether RocksDB-backed EloqDSS should preserve and expose expired
* EloqKV records for diagnostics. Non-RocksDB builds always return false.
*/
inline bool IgnoreRedisTTL()
{
#if defined(DATA_STORE_TYPE_ELOQDSS_ROCKSDB) || \
defined(DATA_STORE_TYPE_ELOQDSS_ROCKSDB_CLOUD_S3) || \
defined(DATA_STORE_TYPE_ELOQDSS_ROCKSDB_CLOUD_GCS)
return FLAGS_ignore_redis_ttl;
#else
return false;
#endif
}
} // namespace EloqDS
10 changes: 10 additions & 0 deletions store_handler/eloq_data_store_service/rocksdb_config.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@
#include "glog/logging.h"

DEFINE_string(rocksdb_info_log_level, "INFO", "RocksDB store info log level");
DEFINE_bool(ignore_redis_ttl,
false,
"Expose persisted EloqKV records without enforcing their TTL. "
"Intended only for diagnostic clusters");
DEFINE_bool(rocksdb_enable_stats, false, "RocksDB store enable stats");
DEFINE_uint32(rocksdb_stats_dump_period_sec,
600,
Expand Down Expand Up @@ -334,6 +338,12 @@ bool CheckCommandLineFlagIsDefault(const char *name)
RocksDBConfig::RocksDBConfig(const INIReader &config,
const std::string &eloq_data_path)
{
if (CheckCommandLineFlagIsDefault("ignore_redis_ttl"))
{
FLAGS_ignore_redis_ttl =
config.GetBoolean("store", "ignore_redis_ttl", false);
}

info_log_level_ = !CheckCommandLineFlagIsDefault("rocksdb_info_log_level")
? FLAGS_rocksdb_info_log_level
: config.GetString("store",
Expand Down
9 changes: 9 additions & 0 deletions store_handler/eloq_data_store_service/rocksdb_data_store_common.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include <cstring>
#include <filesystem>

#include "ignore_redis_ttl.h"
#include "internal_request.h"

namespace EloqDS
Expand Down Expand Up @@ -52,6 +53,14 @@ bool TTLCompactionFilter::Filter(int level,
std::string *new_value,
bool *value_changed) const
{
// Diagnostic mode must not physically reclaim expired records. The value
// remains byte-for-byte unchanged, so normal compaction behavior resumes
// after the mode is disabled and the service is restarted.
if (IgnoreRedisTTL())
{
return false;
}

const DecodedValueHeader header =
DecodeValueHeader(existing_value.data(), existing_value.size());
if (!header.has_ttl)
Expand Down
10 changes: 10 additions & 0 deletions tx_service/tests/TTLCompactionFilter-Test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include <catch2/catch_all.hpp>
// clang-format on

#include "eloq_data_store_service/ignore_redis_ttl.h"
#include "eloq_data_store_service/rocksdb_data_store_common.h"

namespace
Expand Down Expand Up @@ -55,10 +56,19 @@ TEST_CASE(
{
SECTION("expired TTL value is removed")
{
FLAGS_ignore_redis_ttl = false;
const std::string value = MakeValue(EloqDS::MSB | 42, 1);
REQUIRE(ShouldFilter(value, kCompactionTimestamp));
}

SECTION("expired TTL value is retained in diagnostic mode")
{
FLAGS_ignore_redis_ttl = true;
const std::string value = MakeValue(EloqDS::MSB | 42, 1);
REQUIRE_FALSE(ShouldFilter(value, kCompactionTimestamp));
FLAGS_ignore_redis_ttl = false;
}
Comment on lines +59 to +70

@coderabbitai coderabbitai Bot Aug 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Restore FLAGS_ignore_redis_ttl with RAII.

If REQUIRE_FALSE fails on Line 68, Catch2 exits the section before Line 69. Later tests then run with TTL bypass enabled. Preserve and restore the prior flag value with a scoped guard in each section.

Proposed test isolation fix 
+class ScopedIgnoreRedisTTLFlag
+{
+ public:
+    explicit ScopedIgnoreRedisTTLFlag(bool value)
+        : previous_value_(FLAGS_ignore_redis_ttl)
+    {
+        FLAGS_ignore_redis_ttl = value;
+    }
+
+    ~ScopedIgnoreRedisTTLFlag()
+    {
+        FLAGS_ignore_redis_ttl = previous_value_;
+    }
+
+ private:
+    bool previous_value_;
+};
+
-        FLAGS_ignore_redis_ttl = false;
+        ScopedIgnoreRedisTTLFlag ttl_flag(false);
         const std::string value = MakeValue(EloqDS::MSB | 42, 1);
         REQUIRE(ShouldFilter(value, kCompactionTimestamp));
 
-        FLAGS_ignore_redis_ttl = true;
+        ScopedIgnoreRedisTTLFlag ttl_flag(true);
         const std::string value = MakeValue(EloqDS::MSB | 42, 1);
         REQUIRE_FALSE(ShouldFilter(value, kCompactionTimestamp));
-        FLAGS_ignore_redis_ttl = false;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
FLAGS_ignore_redis_ttl = false;
const std::string value = MakeValue(EloqDS::MSB | 42, 1);
REQUIRE(ShouldFilter(value, kCompactionTimestamp));
}
SECTION("expired TTL value is retained in diagnostic mode")
{
FLAGS_ignore_redis_ttl = true;
const std::string value = MakeValue(EloqDS::MSB | 42, 1);
REQUIRE_FALSE(ShouldFilter(value, kCompactionTimestamp));
FLAGS_ignore_redis_ttl = false;
}
class ScopedIgnoreRedisTTLFlag
{
public:
explicit ScopedIgnoreRedisTTLFlag(bool value)
: previous_value_(FLAGS_ignore_redis_ttl)
{
FLAGS_ignore_redis_ttl = value;
}
~ScopedIgnoreRedisTTLFlag()
{
FLAGS_ignore_redis_ttl = previous_value_;
}
private:
bool previous_value_;
};
ScopedIgnoreRedisTTLFlag ttl_flag(false);
const std::string value = MakeValue(EloqDS::MSB | 42, 1);
REQUIRE(ShouldFilter(value, kCompactionTimestamp));
}
SECTION("expired TTL value is retained in diagnostic mode")
{
ScopedIgnoreRedisTTLFlag ttl_flag(true);
const std::string value = MakeValue(EloqDS::MSB | 42, 1);
REQUIRE_FALSE(ShouldFilter(value, kCompactionTimestamp));
}
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tx_service/tests/TTLCompactionFilter-Test.cpp` around lines 59 - 70, Update
both TTL-related sections in the test around ShouldFilter to restore
FLAGS_ignore_redis_ttl via an RAII scoped guard that captures its prior value
and restores it on scope exit, including assertion failures. Remove the manual
reset and preserve each section’s intended flag setting.


SECTION("unexpired TTL value is retained")
{
const std::string value =
Expand Down
Loading