-
Notifications
You must be signed in to change notification settings - Fork 60
feat: support ignoring Redis TTL for diagnostics #555
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -22,6 +22,7 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #pragma once | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #include <algorithm> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #include <atomic> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #include <cstddef> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #include <deque> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #include <memory> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -62,6 +63,22 @@ enum struct RedisObjectType | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| struct RedisEloqObject : public txservice::TxObject | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Controls the diagnostic mode that exposes persisted Redis objects even | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * after their expiration timestamp. The mode changes only runtime TTL | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * interpretation; serialized TTL metadata remains intact so disabling the | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * mode on a later restart restores normal expiration. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| static void SetIgnoreTTL(bool ignore_ttl) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ignore_ttl_.store(ignore_ttl, std::memory_order_release); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| static bool IgnoreTTL() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return ignore_ttl_.load(std::memory_order_acquire); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+66
to
+80
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win Document the startup-only and atomic-state contract.
As per coding guidelines, Proposed documentation /**
- * Controls the diagnostic mode that exposes persisted Redis objects even
- * after their expiration timestamp. The mode changes only runtime TTL
- * interpretation; serialized TTL metadata remains intact so disabling the
- * mode on a later restart restores normal expiration.
+ * Sets the process-wide diagnostic mode. Call this during initialization
+ * before command workers access Redis objects. Do not change the mode
+ * during normal operation.
*/
static void SetIgnoreTTL(bool ignore_ttl)
+ /**
+ * Returns whether diagnostic TTL-ignore mode is enabled.
+ * Atomic access publishes the setting across worker threads but does not
+ * make runtime mode changes semantically consistent.
+ */
static bool IgnoreTTL()📝 Committable suggestion
Suggested change
🤖 Prompt for AI AgentsSource: Coding guidelines |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| TxRecord::Uptr Clone() const override | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert(false); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -128,5 +145,8 @@ struct RedisEloqObject : public txservice::TxObject | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return std::make_unique<RedisEloqObject>(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| inline static std::atomic_bool ignore_ttl_{false}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } // namespace EloqKV | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -58,6 +58,7 @@ | |
| #include "b255.h" | ||
| #include "catalog_factory.h" | ||
| #include "data_substrate.h" | ||
| #include "eloq_data_store_service/ignore_redis_ttl.h" | ||
| #include "eloq_metrics/include/metrics.h" | ||
| #include "eloqkv_key.h" | ||
| #include "error_messages.h" | ||
|
|
@@ -240,6 +241,14 @@ bool RedisServiceImpl::Init(brpc::Server &brpc_server) | |
| return false; | ||
| } | ||
|
|
||
| const bool ignore_redis_ttl = EloqDS::IgnoreRedisTTL(); | ||
| RedisEloqObject::SetIgnoreTTL(ignore_redis_ttl); | ||
| if (ignore_redis_ttl) | ||
| { | ||
| LOG(WARNING) << "ignore_redis_ttl is enabled: persisted expiration " | ||
| "timestamps will be retained but not enforced"; | ||
| } | ||
|
|
||
|
Comment on lines
+244
to
+251
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/usr/bin/env bash
set -euo pipefail
git -C data_substrate grep -n -C 8 -E \
'IgnoreRedisTTL|ignore_redis_ttl|Initialize' -- . || true
git grep -n -C 8 -E \
'RedisServiceImpl::Init|DataSubstrate::Instance|config_file_|ignore_redis_ttl' \
-- src include eloqkv.ini || trueRepository: eloqdata/eloqkv Length of output: 24657 🏁 Script executed: #!/usr/bin/env bash
set -euo pipefail
echo "== locate ignore_redis_ttl =="
rg -n -C 6 'ignore_redis_ttl|IgnoreRedisTTL|SetIgnoreRedisTTL|GetIgnoreRedisTTL' -- . || true
echo "== locate data_substrate init references =="
rg -n -C 5 'DataSubstrate.*Init|Init\(.*config|config.*ignore_redis_ttl|ParseError|CommandLineOption' src include --glob '*.{cpp,cc,c,hpp,h,hxx,cc}' || true
echo "== outline ignore_redis_ttl header/source =="
fd -a 'ignore_redis_ttl\.h|ignore_redis_ttl\.cpp' . || true
for f in $(fd 'ignore_redis_ttl\.(h|cpp)$' .); do echo "--- $f"; ast-grep outline "$f" || true; sed -n '1,240p' "$f"; done
echo "== redis_server startup =="
sed -n '1,120p' src/redis_server.cpp
sed -n '420,475p' src/redis_server.cpp
echo "== redis_service init section =="
sed -n '226,255p' src/redis_service.cppRepository: eloqdata/eloqkv Length of output: 36885 🏁 Script executed: #!/usr/bin/env bash
set -euo pipefail
echo "== data_substrate header/source candidates =="
git ls-files | rg '(^|/)(data_substrate\.(h|cpp|hpp|cc)$|data_substrate/.*)|(eloq_data_store_service/ignore_redis_ttl\.(h|cpp)$)' || true
echo "== exact matches in git tracked files =="
git ls-files | xargs -r grep -n -H -S -C 4 'IgnoreRedisTTL|ignore_redis_ttl|SetIgnoreRedisTTL|GetIgnoreRedisTTL' || true
echo "== precise function/header definitions =="
rg -n -C 8 'bool\s+IgnoreRedisTTL|void\s+SetIgnoreRedisTTL|SET_IGNORE|IGNORE_REDIS|ignore_redis_ttl' . || true
echo "== startup flag parsing candidates =="
rg -n -C 4 'DEFINE_bool.*ignore|GetCommandLineOption|ignore_redis_ttl|ParseCommandLineFlags|SetCommandLineOption' src include --glob '*.{cpp,cc,h,hpp,cc}' || trueRepository: eloqdata/eloqkv Length of output: 29601 Parse
🤖 Prompt for AI Agents |
||
| // Engine registration: EloqKv | ||
| auto &ds = DataSubstrate::Instance(); | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,7 @@ | |
| #include "redis_errors.h" | ||
| #include "redis_hash_object.h" | ||
| #include "redis_list_object.h" | ||
| #include "redis_string_object.h" | ||
| #include "redis_zset_object.h" | ||
|
|
||
| absl::flat_hash_map<std::string_view, double> EloqKV::RedisZsetObject::* | ||
|
|
@@ -28,6 +29,27 @@ struct Rob | |
| } | ||
| }; | ||
|
|
||
| TEST_CASE("ignore Redis TTL preserves serialized metadata") | ||
| { | ||
| EloqKV::RedisStringTTLObject object; | ||
| object.SetTTL(12345); | ||
|
|
||
| EloqKV::RedisEloqObject::SetIgnoreTTL(false); | ||
| REQUIRE(object.HasTTL()); | ||
| std::string before; | ||
| object.Serialize(before); | ||
|
|
||
| EloqKV::RedisEloqObject::SetIgnoreTTL(true); | ||
| REQUIRE_FALSE(object.HasTTL()); | ||
| REQUIRE(object.GetTTL() == 12345); | ||
| std::string ignored; | ||
| object.Serialize(ignored); | ||
| REQUIRE(ignored == before); | ||
|
|
||
| EloqKV::RedisEloqObject::SetIgnoreTTL(false); | ||
| REQUIRE(object.HasTTL()); | ||
| } | ||
|
Comment on lines
+32
to
+51
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
# Locate the test file and relevant implementation symbols.
printf '--- files ---\n'
git ls-files | rg '(^|/)object_serialize_deserialize_test\.cpp$|RedisEloqObject|RedisStringTTLObject|IgnoreTTL|SetIgnoreTTL' || true
printf '\n--- test file excerpt ---\n'
sd='tests/unit/eloq/object_serialize_deserialize_test.cpp'
if [ -f "$sd" ]; then
nl -ba "$sd" | sed -n '1,120p'
fi
printf '\n--- symbol search ---\n'
rg -n "SetIgnoreTTL|IgnoreTTL|class RedisEloqObject|RedisStringTTLObject" . --glob '!build/**' --glob '!dist/**' | head -200Repository: eloqdata/eloqkv Length of output: 244 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Locate the test file and relevant implementation symbols.
echo '--- files ---'
git ls-files | rg '(^|/)object_serialize_deserialize_test\.cpp$|RedisEloqObject|RedisStringTTLObject|IgnoreTTL|SetIgnoreTTL' || true
echo
echo '--- test file excerpt ---'
sd='tests/unit/eloq/object_serialize_deserialize_test.cpp'
if [ -f "$sd" ]; then
nl -ba "$sd" | sed -n '1,120p'
fi
echo
echo '--- symbol search ---'
rg -n "SetIgnoreTTL|IgnoreTTL|class RedisEloqObject|RedisStringTTLObject" . --glob '!build/**' --glob '!dist/**' | head -200Repository: eloqdata/eloqkv Length of output: 289 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '--- test file excerpt ---'
sed -n '1,120p' tests/unit/eloq/object_serialize_deserialize_test.cpp | \
nl -ba
echo
echo '--- symbol search ---'
rg -n "SetIgnoreTTL|IgnoreTTL|class RedisEloqObject|RedisStringTTLObject" . --glob '!build/**' --glob '!dist/**' | head -200
echo
echo '--- definitions/usages ---'
rg -n "SetIgnoreTTL|Ignor(e|Ing)TTL|RedisStringTTLObject::|SetTTL|HasTTL|GetTTL|Serialize\\(" . --glob '!build/**' --glob '!dist/**' | head -300Repository: eloqdata/eloqkv Length of output: 219 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '--- test file excerpt ---'
awk '{printf "%6d\t%s\n", NR, $0}' tests/unit/eloq/object_serialize_deserialize_test.cpp | sed -n '1,160p'
echo
echo '--- symbol search ---'
rg -n "SetIgnoreTTL|IgnoreTTL|class RedisEloqObject|RedisStringTTLObject" . --glob '!build/**' --glob '!dist/**' | head -200
echo
echo '--- TTL serialization definitions/usages ---'
rg -n "SetIgnoreTTL|Ignor(e|Ing)TTL|RedisStringTTLObject::|RedisEloqObject::|SetTTL|HasTTL|GetTTL|Serialize\\(" . --glob '!build/**' --glob '!dist/**' | head -400Repository: eloqdata/eloqkv Length of output: 42758 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '--- redis_object.h relevant section ---'
sed -n '1,130p' include/redis_object.h
echo
echo '--- redis_service.cpp ignore_redis_ttl setup ---'
sed -n '220,255p' src/redis_service.cpp
echo
echo '--- search for SetIgnoreTTL calls ---'
rg -n "SetIgnoreTTL\\(" . --glob '!build/**' --glob '!dist/**'Repository: eloqdata/eloqkv Length of output: 5390 Restore This test writes a process-wide atomic flag after the Catch2 🤖 Prompt for AI Agents |
||
|
|
||
| TEST_CASE("zset_object-string") | ||
| { | ||
| LOG(INFO) << "running: zset_object-string: "; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: eloqdata/eloqkv
Length of output: 21507
🏁 Script executed:
Repository: eloqdata/eloqkv
Length of output: 47193
Preserve TTL semantics while bypassing expiry enforcement.
HasTTL()now reads a process-local flag, butEXPIRE/PERSIST/TTLcommands and checkpoint/TTL-compaction paths useHasTTL()as a metadata presence check. KeepHasTTL()/GetTTL()reflecting the stored TTL, and use the ignore-only flag where expiry enforcement/read/report behavior should be bypassed.📍 Affects 8 files
include/redis_hash_object.h#L268-L270(this comment)include/redis_list_object.h#L264-L266include/redis_set_object.h#L132-L134include/redis_string_object.h#L283-L285include/redis_zset_object.h#L378-L378eloqkv.ini#L101-L107docs/03-data-model.md#L88-L88tests/unit/eloq/object_serialize_deserialize_test.cpp#L39-L47🤖 Prompt for AI Agents