Skip to content

Hash Field Expiration RFC #22

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
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Hash Field Expiration RFC #22

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
63b66b5
Hash Field Expiration RFC
ranshid May 7, 2025
a96c297
refactor to better match the RFC template
ranshid May 8, 2025
a58e32d
Update HFE.md
ranshid May 11, 2025
e9b9775
add AOF, RDB and configuration sections
ranshid May 11, 2025
9c7ba43
fix PR review comment about lazy expiration terminology
ranshid May 11, 2025
3ba5a46
Add Observability section
ranshid May 11, 2025
2f16944
Add RFC header
ranshid May 11, 2025
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
18 changes: 16 additions & 2 deletions HFE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -415,9 +415,23 @@ in caseses were most items are not volatile and the metadata size is small (smal
We suggest to avoid placing new configuration to fine tune this feature. Internal configurations can be discussed in order to be able to
prevent active expiration of volatile items.

## Benchmarking (TBD)
## Observability

## Observability (TBD)
AT the first drop we will only introduce some basic statistics:
1. volatile_items will be added to the Keyspace section per-db line. eg:

```
db0:keys=1,expires=0,avg_ttl=0,volatile_items=16
Copy link
Member

Choose a reason for hiding this comment

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

volatile_items is technically clear, but slightly ambiguous, it might be confused with expires. Maybe:
expire_fields
expiring_fields
volatile_fields

Copy link

Choose a reason for hiding this comment

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

I recommend against modification of this existing INFO line.
This line - containing the number of keys - is one of the most likely lines in INFO to be parsed by a client application (or even a client library). Altering this line has the opportunity to break a large number of existing client applications.

I suggest you add an additional line (or lines) containing the new information.

Copy link
Contributor

Choose a reason for hiding this comment

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

We can hope that clients that parse this split by comma and can handle extra fields. It may break a few but the alternative (a new line per db) may be uglier.

Redis added "subexpiry=0". Can we do the same? We copy their command API...

```

NOTE - we can also consider adding keys_with_volatile_items statistic to track how many objects have
volatile items. eg:

```
db0:keys=1,expires=0,avg_ttl=0,volatile_items=16,keys_with_volatile_items=1
Comment on lines +432 to +436
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't have anything for items currently so it seems fine to me to skip the info about volatile items initially.

If we want it, then I guess we should also add the total number of items (including non-volatile) and the number of keys with items (including non-volatile items).

Regarding the nameing, shouldn't it match the naming used for keys? Here "expires" means the number of volatile keys. To match that, we could use "items", "item_expires" and "item_avg_ttl".

Later, if we add expiration on set elements, sorted set elements, etc. then all these are considered items too, right? Isn't it more useful to have the number of hash fields with expire, set elements with expire and sorted set elements with expire as separate metrics? The full picture starts to look like Wen's KEYSIZES fields: valkey-io/valkey#1967

Copy link
Member Author

Choose a reason for hiding this comment

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

The full picture starts to look like Wen's KEYSIZES fields: valkey-io/valkey#1967

Exactly. I think we will be able to add items statistics and such later on as part of the new KEYS observability

Copy link

Choose a reason for hiding this comment

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

Again, I suggest a new line. Also, in the new line, you can't use a prefix like db0: as this might be searched for explicitly.

Copy link
Contributor

Choose a reason for hiding this comment

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

I just checked what redis does. From their docs of the INFO command, we can see they added "subexpiry":

# Keyspace

db0:keys=112125,expires=456,avg_ttl=31368299122246,subexpiry=0

```

2. expire_items_cycle_cpu_milliseconds statistic will be added to the `stats` and will indicate the number of CPU cycles consumed by the items active expiry cron.

## Design Details

Expand Down