You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@kvrocks.apache.org by "sryanyuan (via GitHub)" <gi...@apache.org> on 2025/09/11 09:32:14 UTC
[PR] feat(config): add RocksDB periodic compaction controls [kvrocks]
sryanyuan opened a new pull request, #3170:
URL: https://github.com/apache/kvrocks/pull/3170
Introduces three new configuration fields for RocksDB periodic compaction management:
1. `rocksdb.periodic_compaction_seconds` - Sets compaction frequency
2. `rocksdb.ttl` - Enables TTL-based compaction
3. `rocksdb.daily_offpeak_time_utc` - Configures off-peak compaction window
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(config): add RocksDB periodic compaction controls [kvrocks]
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on PR #3170:
URL: https://github.com/apache/kvrocks/pull/3170#issuecomment-3290217062
@mapleFU could you help review since it is highly rocksdb related?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(config): add RocksDB periodic compaction controls [kvrocks]
Posted by "sryanyuan (via GitHub)" <gi...@apache.org>.
sryanyuan commented on code in PR #3170:
URL: https://github.com/apache/kvrocks/pull/3170#discussion_r2350489123
##########
kvrocks.conf:
##########
@@ -1124,5 +1124,54 @@ rocksdb.max_compaction_bytes 0
# Default: 0
rocksdb.sst_file_delete_rate_bytes_per_sec 0
+# Enable RocksDB periodic compaction to force full compaction of SST files older than the specified time (in seconds).
+# If a compaction filter is registered, it will be applied during these compactions.
+# Set to 0 to disable this feature.
+#
+# Default: 18446744073709551614 (0xFFFFFFFFFFFFFFFE, UINT64_MAX - 1), a special value indicating RocksDB-controlled behavior.
+# Currently, RocksDB interprets this default as 30 days (2592000 seconds).
+#
+# Typical use cases:
+# - Enforcing data cleanup via compaction filters (e.g., TTL expiration)
+# - Automatically refreshing data encoding/compression formats without manual intervention
+#
+# Reference: https://github.com/facebook/rocksdb/wiki/Leveled-Compaction#periodic-compaction
+rocksdb.periodic_compaction_seconds
+
+# Enable RocksDB Time-to-Live (TTL) to automatically schedule compaction for SST files containing expired data.
+# - Files containing data older than the TTL (in seconds) will be prioritized for background compaction.
+# - Requires a registered compaction filter (e.g., TTL filter) to identify and remove expired entries.
+# - Set to 0 to disable TTL-based compaction.
+#
+# Default: 18446744073709551614 (0xFFFFFFFFFFFFFFFE, UINT64_MAX - 1), delegating control to RocksDB.
+# Current RocksDB behavior interprets this default as 30 days (2592000 seconds).
+#
+# Use cases:
+# - Automatic expiration of ephemeral data (e.g., session tokens, temporary logs)
+# - Lifecycle management for time-series datasets
+#
+# Reference: https://github.com/facebook/rocksdb/wiki/Leveled-Compaction#ttl
+rocksdb.ttl
Review Comment:
Updated
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(config): add RocksDB periodic compaction controls [kvrocks]
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #3170:
URL: https://github.com/apache/kvrocks/pull/3170#discussion_r2339782887
##########
src/config/config.cc:
##########
@@ -125,8 +125,8 @@ Status SetRocksdbCompression(Server *srv, const rocksdb::CompressionType compres
for (size_t i = compression_start_level; i < KVROCKS_MAX_LSM_LEVEL; i++) {
compression_per_level_builder.emplace_back(compression_option);
}
- const std::string compression_per_level = util::StringJoin(
- compression_per_level_builder, [](const auto &s) -> decltype(auto) { return s; }, ":");
+ const std::string compression_per_level =
+ util::StringJoin(compression_per_level_builder, [](const auto &s) -> decltype(auto) { return s; }, ":");
Review Comment:
```suggestion
const std::string compression_per_level = util::StringJoin(compression_per_level_builder, ":");
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(config): add RocksDB periodic compaction controls [kvrocks]
Posted by "sryanyuan (via GitHub)" <gi...@apache.org>.
sryanyuan commented on code in PR #3170:
URL: https://github.com/apache/kvrocks/pull/3170#discussion_r2347798280
##########
src/config/config.cc:
##########
@@ -125,8 +125,8 @@ Status SetRocksdbCompression(Server *srv, const rocksdb::CompressionType compres
for (size_t i = compression_start_level; i < KVROCKS_MAX_LSM_LEVEL; i++) {
compression_per_level_builder.emplace_back(compression_option);
}
- const std::string compression_per_level = util::StringJoin(
- compression_per_level_builder, [](const auto &s) -> decltype(auto) { return s; }, ":");
+ const std::string compression_per_level =
+ util::StringJoin(compression_per_level_builder, [](const auto &s) -> decltype(auto) { return s; }, ":");
Review Comment:
I do not change the code, just format it. Change to new code causes compile error:
##########
src/config/config.cc:
##########
@@ -125,8 +125,8 @@ Status SetRocksdbCompression(Server *srv, const rocksdb::CompressionType compres
for (size_t i = compression_start_level; i < KVROCKS_MAX_LSM_LEVEL; i++) {
compression_per_level_builder.emplace_back(compression_option);
}
- const std::string compression_per_level = util::StringJoin(
- compression_per_level_builder, [](const auto &s) -> decltype(auto) { return s; }, ":");
+ const std::string compression_per_level =
+ util::StringJoin(compression_per_level_builder, [](const auto &s) -> decltype(auto) { return s; }, ":");
Review Comment:
I do not change the code, just format it. Change to new code causes compile error:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(config): add RocksDB periodic compaction controls [kvrocks]
Posted by "sryanyuan (via GitHub)" <gi...@apache.org>.
sryanyuan commented on code in PR #3170:
URL: https://github.com/apache/kvrocks/pull/3170#discussion_r2348004995
##########
src/config/config.cc:
##########
@@ -307,6 +307,10 @@ Config::Config() {
{"rocksdb.max_compaction_bytes", false, new Int64Field(&rocks_db.max_compaction_bytes, 0, 0, INT64_MAX)},
{"rocksdb.sst_file_delete_rate_bytes_per_sec", false,
new Int64Field(&rocks_db.sst_file_delete_rate_bytes_per_sec, 0, 0, INT64_MAX)},
+ {"rocksdb.periodic_compaction_seconds", false,
+ new UInt64Field(&rocks_db.periodic_compaction_seconds, kDefaultRocksdbPeriodicCompactionSeconds, 0, UINT64_MAX)},
+ {"rocksdb.ttl", false, new UInt64Field(&rocks_db.ttl, kDefaultRocksdbTTL, 0, UINT64_MAX)},
Review Comment:
The default values `kDefaultRocksdbPeriodicCompactionSeconds` and `kDefaultRocksdbTTL` are set to `UINT64_MAX-1`, consistent with RocksDB's native defaults. Leaving them unconfigured preserves the TTL/periodic compaction behaviors. Do you mean changing those config types to string and accepting the "default" value? When "default" is set, do not pass the Kvrocks config value to RocksDB.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(config): add RocksDB periodic compaction controls [kvrocks]
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #3170:
URL: https://github.com/apache/kvrocks/pull/3170#discussion_r2339841818
##########
kvrocks.conf:
##########
@@ -1124,5 +1124,54 @@ rocksdb.max_compaction_bytes 0
# Default: 0
rocksdb.sst_file_delete_rate_bytes_per_sec 0
+# Enable RocksDB periodic compaction to force full compaction of SST files older than the specified time (in seconds).
+# If a compaction filter is registered, it will be applied during these compactions.
+# Set to 0 to disable this feature.
+#
+# Default: 18446744073709551614 (UINT64_MAX - 1), a special value indicating RocksDB-controlled behavior.
+# Currently, RocksDB interprets this default as 30 days (2592000 seconds).
+#
+# Typical use cases:
+# - Enforcing data cleanup via compaction filters (e.g., TTL expiration)
+# - Automatically refreshing data encoding/compression formats without manual intervention
+#
+# Reference: https://github.com/facebook/rocksdb/wiki/Periodic-Compaction
+rocksdb.periodic_compaction_seconds 18446744073709551614
+
+# Enable RocksDB Time-to-Live (TTL) to automatically schedule compaction for SST files containing expired data.
+# - Files containing data older than the TTL (in seconds) will be prioritized for background compaction.
+# - Requires a registered compaction filter (e.g., TTL filter) to identify and remove expired entries.
+# - Set to 0 to disable TTL-based compaction.
+#
+# Default: 18446744073709551614 (0xFFFFFFFFFFFFFFFE, UINT64_MAX - 1), delegating control to RocksDB.
+# Current RocksDB behavior interprets this default as 30 days (2592000 seconds).
+#
+# Use cases:
+# - Automatic expiration of ephemeral data (e.g., session tokens, temporary logs)
+# - Lifecycle management for time-series datasets
+#
+# Reference: https://github.com/facebook/rocksdb/wiki/Leveled-Compaction#ttl
+rocksdb.ttl 18446744073709551614
Review Comment:
is `ttl` required by `periodic_compaction_seconds`? any dependency here? it seems that we didn't use this feature yet in kvrocks.
also it is enabled by default here which may cause issues?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(config): add RocksDB periodic compaction controls [kvrocks]
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #3170:
URL: https://github.com/apache/kvrocks/pull/3170#discussion_r2349294737
##########
kvrocks.conf:
##########
@@ -1124,5 +1124,54 @@ rocksdb.max_compaction_bytes 0
# Default: 0
rocksdb.sst_file_delete_rate_bytes_per_sec 0
+# Enable RocksDB periodic compaction to force full compaction of SST files older than the specified time (in seconds).
+# If a compaction filter is registered, it will be applied during these compactions.
+# Set to 0 to disable this feature.
+#
+# Default: 18446744073709551614 (0xFFFFFFFFFFFFFFFE, UINT64_MAX - 1), a special value indicating RocksDB-controlled behavior.
+# Currently, RocksDB interprets this default as 30 days (2592000 seconds).
+#
+# Typical use cases:
+# - Enforcing data cleanup via compaction filters (e.g., TTL expiration)
+# - Automatically refreshing data encoding/compression formats without manual intervention
+#
+# Reference: https://github.com/facebook/rocksdb/wiki/Leveled-Compaction#periodic-compaction
+rocksdb.periodic_compaction_seconds
+
+# Enable RocksDB Time-to-Live (TTL) to automatically schedule compaction for SST files containing expired data.
+# - Files containing data older than the TTL (in seconds) will be prioritized for background compaction.
+# - Requires a registered compaction filter (e.g., TTL filter) to identify and remove expired entries.
+# - Set to 0 to disable TTL-based compaction.
+#
+# Default: 18446744073709551614 (0xFFFFFFFFFFFFFFFE, UINT64_MAX - 1), delegating control to RocksDB.
+# Current RocksDB behavior interprets this default as 30 days (2592000 seconds).
+#
+# Use cases:
+# - Automatic expiration of ephemeral data (e.g., session tokens, temporary logs)
+# - Lifecycle management for time-series datasets
+#
+# Reference: https://github.com/facebook/rocksdb/wiki/Leveled-Compaction#ttl
+rocksdb.ttl
+
+# Schedule RocksDB periodic compactions during daily off-peak windows to reduce operational impact.
+#
+# Requirements:
+# - Periodic compaction must be enabled (`periodic-compaction-seconds > 0`)
+# - Time format: "HH:MM-HH:MM" in UTC (e.g., "02:00-04:30" for a 2.5-hour window)
+# - Empty string disables off-peak scheduling
+#
+# Behavior:
+# - RocksDB proactively triggers periodic compactions during the specified off-peak window
+# - Compactions are optimized to complete before the next peak period begins
+#
+# Default: "" (disabled)
+#
+# Typical use cases:
+# - Minimize compaction I/O during business hours for latency-sensitive workloads
+# - Align resource-heavy operations with maintenance windows
+#
+# Reference: https://github.com/facebook/rocksdb/wiki/Daily-Off%E2%80%90peak-Time-Option
+rocksdb.daily_offpeak_time_utc
Review Comment:
```suggestion
rocksdb.daily_offpeak_time_utc ""
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(config): add RocksDB periodic compaction controls [kvrocks]
Posted by "sryanyuan (via GitHub)" <gi...@apache.org>.
sryanyuan commented on code in PR #3170:
URL: https://github.com/apache/kvrocks/pull/3170#discussion_r2347755436
##########
kvrocks.conf:
##########
@@ -1124,5 +1124,54 @@ rocksdb.max_compaction_bytes 0
# Default: 0
rocksdb.sst_file_delete_rate_bytes_per_sec 0
+# Enable RocksDB periodic compaction to force full compaction of SST files older than the specified time (in seconds).
+# If a compaction filter is registered, it will be applied during these compactions.
+# Set to 0 to disable this feature.
+#
+# Default: 18446744073709551614 (UINT64_MAX - 1), a special value indicating RocksDB-controlled behavior.
+# Currently, RocksDB interprets this default as 30 days (2592000 seconds).
+#
+# Typical use cases:
+# - Enforcing data cleanup via compaction filters (e.g., TTL expiration)
+# - Automatically refreshing data encoding/compression formats without manual intervention
+#
+# Reference: https://github.com/facebook/rocksdb/wiki/Periodic-Compaction
Review Comment:
Updated
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(config): add RocksDB periodic compaction controls [kvrocks]
Posted by "mapleFU (via GitHub)" <gi...@apache.org>.
mapleFU commented on code in PR #3170:
URL: https://github.com/apache/kvrocks/pull/3170#discussion_r2348013042
##########
src/config/config.cc:
##########
@@ -307,6 +307,10 @@ Config::Config() {
{"rocksdb.max_compaction_bytes", false, new Int64Field(&rocks_db.max_compaction_bytes, 0, 0, INT64_MAX)},
{"rocksdb.sst_file_delete_rate_bytes_per_sec", false,
new Int64Field(&rocks_db.sst_file_delete_rate_bytes_per_sec, 0, 0, INT64_MAX)},
+ {"rocksdb.periodic_compaction_seconds", false,
+ new UInt64Field(&rocks_db.periodic_compaction_seconds, kDefaultRocksdbPeriodicCompactionSeconds, 0, UINT64_MAX)},
+ {"rocksdb.ttl", false, new UInt64Field(&rocks_db.ttl, kDefaultRocksdbTTL, 0, UINT64_MAX)},
Review Comment:
Ah, I got this
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(config): add RocksDB periodic compaction controls [kvrocks]
Posted by "sryanyuan (via GitHub)" <gi...@apache.org>.
sryanyuan commented on code in PR #3170:
URL: https://github.com/apache/kvrocks/pull/3170#discussion_r2347798411
##########
src/config/config.cc:
##########
@@ -125,8 +125,8 @@ Status SetRocksdbCompression(Server *srv, const rocksdb::CompressionType compres
for (size_t i = compression_start_level; i < KVROCKS_MAX_LSM_LEVEL; i++) {
compression_per_level_builder.emplace_back(compression_option);
}
- const std::string compression_per_level = util::StringJoin(
- compression_per_level_builder, [](const auto &s) -> decltype(auto) { return s; }, ":");
+ const std::string compression_per_level =
+ util::StringJoin(compression_per_level_builder, [](const auto &s) -> decltype(auto) { return s; }, ":");
Review Comment:
I do not change the code, just format it. Change to new code causes compile error:
##########
src/config/config.cc:
##########
@@ -125,8 +125,8 @@ Status SetRocksdbCompression(Server *srv, const rocksdb::CompressionType compres
for (size_t i = compression_start_level; i < KVROCKS_MAX_LSM_LEVEL; i++) {
compression_per_level_builder.emplace_back(compression_option);
}
- const std::string compression_per_level = util::StringJoin(
- compression_per_level_builder, [](const auto &s) -> decltype(auto) { return s; }, ":");
+ const std::string compression_per_level =
+ util::StringJoin(compression_per_level_builder, [](const auto &s) -> decltype(auto) { return s; }, ":");
Review Comment:
I do not change the code, just format it. Change to new code causes compile error:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(config): add RocksDB periodic compaction controls [kvrocks]
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #3170:
URL: https://github.com/apache/kvrocks/pull/3170#discussion_r2347809377
##########
src/config/config.cc:
##########
@@ -125,8 +125,8 @@ Status SetRocksdbCompression(Server *srv, const rocksdb::CompressionType compres
for (size_t i = compression_start_level; i < KVROCKS_MAX_LSM_LEVEL; i++) {
compression_per_level_builder.emplace_back(compression_option);
}
- const std::string compression_per_level = util::StringJoin(
- compression_per_level_builder, [](const auto &s) -> decltype(auto) { return s; }, ":");
+ const std::string compression_per_level =
+ util::StringJoin(compression_per_level_builder, [](const auto &s) -> decltype(auto) { return s; }, ":");
Review Comment:
You can ignore this change. I'll fix it soon.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(config): add RocksDB periodic compaction controls [kvrocks]
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #3170:
URL: https://github.com/apache/kvrocks/pull/3170#discussion_r2339817487
##########
kvrocks.conf:
##########
@@ -1124,5 +1124,54 @@ rocksdb.max_compaction_bytes 0
# Default: 0
rocksdb.sst_file_delete_rate_bytes_per_sec 0
+# Enable RocksDB periodic compaction to force full compaction of SST files older than the specified time (in seconds).
+# If a compaction filter is registered, it will be applied during these compactions.
+# Set to 0 to disable this feature.
+#
+# Default: 18446744073709551614 (UINT64_MAX - 1), a special value indicating RocksDB-controlled behavior.
Review Comment:
I prefer `-1` than this value, as it is not quite friendly to users.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(config): add RocksDB periodic compaction controls [kvrocks]
Posted by "sryanyuan (via GitHub)" <gi...@apache.org>.
sryanyuan commented on code in PR #3170:
URL: https://github.com/apache/kvrocks/pull/3170#discussion_r2347745772
##########
kvrocks.conf:
##########
@@ -1124,5 +1124,54 @@ rocksdb.max_compaction_bytes 0
# Default: 0
rocksdb.sst_file_delete_rate_bytes_per_sec 0
+# Enable RocksDB periodic compaction to force full compaction of SST files older than the specified time (in seconds).
+# If a compaction filter is registered, it will be applied during these compactions.
+# Set to 0 to disable this feature.
+#
+# Default: 18446744073709551614 (UINT64_MAX - 1), a special value indicating RocksDB-controlled behavior.
+# Currently, RocksDB interprets this default as 30 days (2592000 seconds).
+#
+# Typical use cases:
+# - Enforcing data cleanup via compaction filters (e.g., TTL expiration)
+# - Automatically refreshing data encoding/compression formats without manual intervention
+#
+# Reference: https://github.com/facebook/rocksdb/wiki/Periodic-Compaction
Review Comment:
:( I will correct it soon.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(config): add RocksDB periodic compaction controls [kvrocks]
Posted by "sryanyuan (via GitHub)" <gi...@apache.org>.
sryanyuan commented on code in PR #3170:
URL: https://github.com/apache/kvrocks/pull/3170#discussion_r2347745507
##########
src/config/config.cc:
##########
@@ -307,6 +306,10 @@ Config::Config() {
{"rocksdb.max_compaction_bytes", false, new Int64Field(&rocks_db.max_compaction_bytes, 0, 0, INT64_MAX)},
{"rocksdb.sst_file_delete_rate_bytes_per_sec", false,
new Int64Field(&rocks_db.sst_file_delete_rate_bytes_per_sec, 0, 0, INT64_MAX)},
+ {"rocksdb.periodic_compaction_seconds", false,
+ new UInt64Field(&rocks_db.periodic_compaction_seconds, UINT64_MAX - 1, 0, UINT64_MAX)},
+ {"rocksdb.ttl", false, new UInt64Field(&rocks_db.ttl, UINT64_MAX - 1, 0, UINT64_MAX)},
Review Comment:
OK
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(config): add RocksDB periodic compaction controls [kvrocks]
Posted by "mapleFU (via GitHub)" <gi...@apache.org>.
mapleFU commented on code in PR #3170:
URL: https://github.com/apache/kvrocks/pull/3170#discussion_r2347739275
##########
kvrocks.conf:
##########
@@ -1124,5 +1124,54 @@ rocksdb.max_compaction_bytes 0
# Default: 0
rocksdb.sst_file_delete_rate_bytes_per_sec 0
+# Enable RocksDB periodic compaction to force full compaction of SST files older than the specified time (in seconds).
+# If a compaction filter is registered, it will be applied during these compactions.
+# Set to 0 to disable this feature.
+#
+# Default: 18446744073709551614 (UINT64_MAX - 1), a special value indicating RocksDB-controlled behavior.
+# Currently, RocksDB interprets this default as 30 days (2592000 seconds).
+#
+# Typical use cases:
+# - Enforcing data cleanup via compaction filters (e.g., TTL expiration)
+# - Automatically refreshing data encoding/compression formats without manual intervention
+#
+# Reference: https://github.com/facebook/rocksdb/wiki/Periodic-Compaction
Review Comment:
I can only visit https://github.com/facebook/rocksdb/wiki/Leveled-Compaction#periodic-compaction , is this expected?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(config): add RocksDB periodic compaction controls [kvrocks]
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #3170:
URL: https://github.com/apache/kvrocks/pull/3170#discussion_r2347757194
##########
kvrocks.conf:
##########
@@ -1124,5 +1124,54 @@ rocksdb.max_compaction_bytes 0
# Default: 0
rocksdb.sst_file_delete_rate_bytes_per_sec 0
+# Enable RocksDB periodic compaction to force full compaction of SST files older than the specified time (in seconds).
+# If a compaction filter is registered, it will be applied during these compactions.
+# Set to 0 to disable this feature.
+#
+# Default: 18446744073709551614 (UINT64_MAX - 1), a special value indicating RocksDB-controlled behavior.
Review Comment:
if we don't want to use `-1` here, we can try something like `default` as a string.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(config): add RocksDB periodic compaction controls [kvrocks]
Posted by "mapleFU (via GitHub)" <gi...@apache.org>.
mapleFU commented on code in PR #3170:
URL: https://github.com/apache/kvrocks/pull/3170#discussion_r2347975442
##########
kvrocks.conf:
##########
@@ -1124,5 +1124,54 @@ rocksdb.max_compaction_bytes 0
# Default: 0
rocksdb.sst_file_delete_rate_bytes_per_sec 0
+# Enable RocksDB periodic compaction to force full compaction of SST files older than the specified time (in seconds).
+# If a compaction filter is registered, it will be applied during these compactions.
+# Set to 0 to disable this feature.
+#
+# Default: 18446744073709551614 (0xFFFFFFFFFFFFFFFE, UINT64_MAX - 1), a special value indicating RocksDB-controlled behavior.
+# Currently, RocksDB interprets this default as 30 days (2592000 seconds).
+#
+# Typical use cases:
+# - Enforcing data cleanup via compaction filters (e.g., TTL expiration)
+# - Automatically refreshing data encoding/compression formats without manual intervention
+#
+# Reference: https://github.com/facebook/rocksdb/wiki/Leveled-Compaction#periodic-compaction
+rocksdb.periodic_compaction_seconds
+
+# Enable RocksDB Time-to-Live (TTL) to automatically schedule compaction for SST files containing expired data.
+# - Files containing data older than the TTL (in seconds) will be prioritized for background compaction.
+# - Requires a registered compaction filter (e.g., TTL filter) to identify and remove expired entries.
+# - Set to 0 to disable TTL-based compaction.
+#
+# Default: 18446744073709551614 (0xFFFFFFFFFFFFFFFE, UINT64_MAX - 1), delegating control to RocksDB.
Review Comment:
Wrong default?
##########
src/config/config.cc:
##########
@@ -307,6 +307,10 @@ Config::Config() {
{"rocksdb.max_compaction_bytes", false, new Int64Field(&rocks_db.max_compaction_bytes, 0, 0, INT64_MAX)},
{"rocksdb.sst_file_delete_rate_bytes_per_sec", false,
new Int64Field(&rocks_db.sst_file_delete_rate_bytes_per_sec, 0, 0, INT64_MAX)},
+ {"rocksdb.periodic_compaction_seconds", false,
+ new UInt64Field(&rocks_db.periodic_compaction_seconds, kDefaultRocksdbPeriodicCompactionSeconds, 0, UINT64_MAX)},
+ {"rocksdb.ttl", false, new UInt64Field(&rocks_db.ttl, kDefaultRocksdbTTL, 0, UINT64_MAX)},
Review Comment:
These are not "default"?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(config): add RocksDB periodic compaction controls [kvrocks]
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #3170:
URL: https://github.com/apache/kvrocks/pull/3170#discussion_r2349293664
##########
kvrocks.conf:
##########
@@ -1124,5 +1124,54 @@ rocksdb.max_compaction_bytes 0
# Default: 0
rocksdb.sst_file_delete_rate_bytes_per_sec 0
+# Enable RocksDB periodic compaction to force full compaction of SST files older than the specified time (in seconds).
+# If a compaction filter is registered, it will be applied during these compactions.
+# Set to 0 to disable this feature.
+#
+# Default: 18446744073709551614 (0xFFFFFFFFFFFFFFFE, UINT64_MAX - 1), a special value indicating RocksDB-controlled behavior.
+# Currently, RocksDB interprets this default as 30 days (2592000 seconds).
+#
+# Typical use cases:
+# - Enforcing data cleanup via compaction filters (e.g., TTL expiration)
+# - Automatically refreshing data encoding/compression formats without manual intervention
+#
+# Reference: https://github.com/facebook/rocksdb/wiki/Leveled-Compaction#periodic-compaction
+rocksdb.periodic_compaction_seconds
Review Comment:
same as above.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(config): add RocksDB periodic compaction controls [kvrocks]
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #3170:
URL: https://github.com/apache/kvrocks/pull/3170#discussion_r2349262901
##########
kvrocks.conf:
##########
@@ -1124,5 +1124,54 @@ rocksdb.max_compaction_bytes 0
# Default: 0
rocksdb.sst_file_delete_rate_bytes_per_sec 0
+# Enable RocksDB periodic compaction to force full compaction of SST files older than the specified time (in seconds).
+# If a compaction filter is registered, it will be applied during these compactions.
+# Set to 0 to disable this feature.
+#
+# Default: 18446744073709551614 (0xFFFFFFFFFFFFFFFE, UINT64_MAX - 1), a special value indicating RocksDB-controlled behavior.
+# Currently, RocksDB interprets this default as 30 days (2592000 seconds).
+#
+# Typical use cases:
+# - Enforcing data cleanup via compaction filters (e.g., TTL expiration)
+# - Automatically refreshing data encoding/compression formats without manual intervention
+#
+# Reference: https://github.com/facebook/rocksdb/wiki/Leveled-Compaction#periodic-compaction
+rocksdb.periodic_compaction_seconds
+
+# Enable RocksDB Time-to-Live (TTL) to automatically schedule compaction for SST files containing expired data.
+# - Files containing data older than the TTL (in seconds) will be prioritized for background compaction.
+# - Requires a registered compaction filter (e.g., TTL filter) to identify and remove expired entries.
+# - Set to 0 to disable TTL-based compaction.
+#
+# Default: 18446744073709551614 (0xFFFFFFFFFFFFFFFE, UINT64_MAX - 1), delegating control to RocksDB.
+# Current RocksDB behavior interprets this default as 30 days (2592000 seconds).
+#
+# Use cases:
+# - Automatic expiration of ephemeral data (e.g., session tokens, temporary logs)
+# - Lifecycle management for time-series datasets
+#
+# Reference: https://github.com/facebook/rocksdb/wiki/Leveled-Compaction#ttl
+rocksdb.ttl
Review Comment:
instead we can just make it a comment.
```suggestion
# rocksdb.ttl <some example value>
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(config): add RocksDB periodic compaction controls [kvrocks]
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #3170:
URL: https://github.com/apache/kvrocks/pull/3170#discussion_r2339829837
##########
kvrocks.conf:
##########
@@ -1124,5 +1124,54 @@ rocksdb.max_compaction_bytes 0
# Default: 0
rocksdb.sst_file_delete_rate_bytes_per_sec 0
+# Enable RocksDB periodic compaction to force full compaction of SST files older than the specified time (in seconds).
+# If a compaction filter is registered, it will be applied during these compactions.
+# Set to 0 to disable this feature.
+#
+# Default: 18446744073709551614 (UINT64_MAX - 1), a special value indicating RocksDB-controlled behavior.
+# Currently, RocksDB interprets this default as 30 days (2592000 seconds).
+#
+# Typical use cases:
+# - Enforcing data cleanup via compaction filters (e.g., TTL expiration)
+# - Automatically refreshing data encoding/compression formats without manual intervention
+#
+# Reference: https://github.com/facebook/rocksdb/wiki/Periodic-Compaction
+rocksdb.periodic_compaction_seconds 18446744073709551614
Review Comment:
I'm not sure if we should enable this by default, as it could do compaction out of the cron time.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(config): add RocksDB periodic compaction controls [kvrocks]
Posted by "mapleFU (via GitHub)" <gi...@apache.org>.
mapleFU commented on code in PR #3170:
URL: https://github.com/apache/kvrocks/pull/3170#discussion_r2347745363
##########
src/config/config.cc:
##########
@@ -307,6 +306,10 @@ Config::Config() {
{"rocksdb.max_compaction_bytes", false, new Int64Field(&rocks_db.max_compaction_bytes, 0, 0, INT64_MAX)},
{"rocksdb.sst_file_delete_rate_bytes_per_sec", false,
new Int64Field(&rocks_db.sst_file_delete_rate_bytes_per_sec, 0, 0, INT64_MAX)},
+ {"rocksdb.periodic_compaction_seconds", false,
+ new UInt64Field(&rocks_db.periodic_compaction_seconds, UINT64_MAX - 1, 0, UINT64_MAX)},
+ {"rocksdb.ttl", false, new UInt64Field(&rocks_db.ttl, UINT64_MAX - 1, 0, UINT64_MAX)},
Review Comment:
Can we add two constant DefaultTtl and DefaultPeriodicCompSecs? `UINT64_MAX - 1` seems two magic numbers
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(config): add RocksDB periodic compaction controls [kvrocks]
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice merged PR #3170:
URL: https://github.com/apache/kvrocks/pull/3170
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(config): add RocksDB periodic compaction controls [kvrocks]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3170:
URL: https://github.com/apache/kvrocks/pull/3170#issuecomment-3297299264
## [](https://sonarcloud.io/dashboard?id=apache_kvrocks&pullRequest=3170) **Quality Gate passed**
Issues
 [19 New issues](https://sonarcloud.io/project/issues?id=apache_kvrocks&pullRequest=3170&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_kvrocks&pullRequest=3170&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_kvrocks&pullRequest=3170&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [64.8% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_kvrocks&pullRequest=3170&metric=new_coverage&view=list)
 [2.7% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_kvrocks&pullRequest=3170&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_kvrocks&pullRequest=3170)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(config): add RocksDB periodic compaction controls [kvrocks]
Posted by "sryanyuan (via GitHub)" <gi...@apache.org>.
sryanyuan commented on code in PR #3170:
URL: https://github.com/apache/kvrocks/pull/3170#discussion_r2347731077
##########
kvrocks.conf:
##########
@@ -1124,5 +1124,54 @@ rocksdb.max_compaction_bytes 0
# Default: 0
rocksdb.sst_file_delete_rate_bytes_per_sec 0
+# Enable RocksDB periodic compaction to force full compaction of SST files older than the specified time (in seconds).
+# If a compaction filter is registered, it will be applied during these compactions.
+# Set to 0 to disable this feature.
+#
+# Default: 18446744073709551614 (UINT64_MAX - 1), a special value indicating RocksDB-controlled behavior.
+# Currently, RocksDB interprets this default as 30 days (2592000 seconds).
+#
+# Typical use cases:
+# - Enforcing data cleanup via compaction filters (e.g., TTL expiration)
+# - Automatically refreshing data encoding/compression formats without manual intervention
+#
+# Reference: https://github.com/facebook/rocksdb/wiki/Periodic-Compaction
+rocksdb.periodic_compaction_seconds 18446744073709551614
Review Comment:
Kvrocks uses the default RocksDB periodic compaction. The default value is set to (UINT64_MAX - 1) (equivalent to 30 days), which enables the periodic compaction feature by default.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(config): add RocksDB periodic compaction controls [kvrocks]
Posted by "sryanyuan (via GitHub)" <gi...@apache.org>.
sryanyuan commented on code in PR #3170:
URL: https://github.com/apache/kvrocks/pull/3170#discussion_r2347761841
##########
kvrocks.conf:
##########
@@ -1124,5 +1124,54 @@ rocksdb.max_compaction_bytes 0
# Default: 0
rocksdb.sst_file_delete_rate_bytes_per_sec 0
+# Enable RocksDB periodic compaction to force full compaction of SST files older than the specified time (in seconds).
+# If a compaction filter is registered, it will be applied during these compactions.
+# Set to 0 to disable this feature.
+#
+# Default: 18446744073709551614 (UINT64_MAX - 1), a special value indicating RocksDB-controlled behavior.
Review Comment:
I'll work on improving it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(config): add RocksDB periodic compaction controls [kvrocks]
Posted by "sryanyuan (via GitHub)" <gi...@apache.org>.
sryanyuan commented on code in PR #3170:
URL: https://github.com/apache/kvrocks/pull/3170#discussion_r2347800333
##########
src/config/config.cc:
##########
@@ -125,8 +125,8 @@ Status SetRocksdbCompression(Server *srv, const rocksdb::CompressionType compres
for (size_t i = compression_start_level; i < KVROCKS_MAX_LSM_LEVEL; i++) {
compression_per_level_builder.emplace_back(compression_option);
}
- const std::string compression_per_level = util::StringJoin(
- compression_per_level_builder, [](const auto &s) -> decltype(auto) { return s; }, ":");
+ const std::string compression_per_level =
+ util::StringJoin(compression_per_level_builder, [](const auto &s) -> decltype(auto) { return s; }, ":");
Review Comment:
I did not modify the code logic, but only reformatted it. Attempting to introduce new code triggers a compilation error: called object type 'const char[2]' is not a function or function pointer
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(config): add RocksDB periodic compaction controls [kvrocks]
Posted by "sryanyuan (via GitHub)" <gi...@apache.org>.
sryanyuan commented on code in PR #3170:
URL: https://github.com/apache/kvrocks/pull/3170#discussion_r2347833632
##########
kvrocks.conf:
##########
@@ -1124,5 +1124,54 @@ rocksdb.max_compaction_bytes 0
# Default: 0
rocksdb.sst_file_delete_rate_bytes_per_sec 0
+# Enable RocksDB periodic compaction to force full compaction of SST files older than the specified time (in seconds).
+# If a compaction filter is registered, it will be applied during these compactions.
+# Set to 0 to disable this feature.
+#
+# Default: 18446744073709551614 (UINT64_MAX - 1), a special value indicating RocksDB-controlled behavior.
Review Comment:
Leaving those configuration parameters undefined now
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org