bitflicker64 opened a new pull request, #3382:
URL: https://github.com/apache/hugegraph/pull/3382
## Purpose of the PR
- fix #3378
`KvService.clearTTLData` runs every second on the PD leader and should
delete each key whose TTL
marker `K@T@<expireAt>@<key>` has expired. It took the key from the marker
at the delimiter after
`T`, so it deleted `<expireAt>@<key>`, a name that never exists, and then
removed the marker. A
read hides an expired value and removes it when it finds one, but a value
that nobody reads again
stayed in the store for good, with no marker left that would ever delete it.
#3378 has the
measurement on master.
The parse fix alone would make the cleanup delete a key that a writer is
renewing at that moment,
which the review of #3376 asked to prevent: the cleanup's delete should be
bound to the expired
marker. This PR carries the parse fix and that guard without the `txn` RPC
of #3376, so the guard
is a check before the delete, not one atomic raft entry. Once that RPC
lands, the check and the
delete can become one guarded txn; its two compares are the two checks below.
This PR is limited to the TTL cleanup's parse and its guard; a bug that
already exists on master in
code this PR did not write is tracked as an issue, not fixed here.
## Main Changes
One commit, two files, 54 production lines (51 added, 3 removed) and 113
test lines.
`clearTTLData` parses the key after the marker's time, reads the key, and
hands both to the new
`KvService.removeExpired(key, marker, stored)`, which deletes the key and
the marker only while the
key still holds the value the marker names:
- The value is absent (a read removed it and left the marker behind, point 4
of #3378): only the
marker goes.
- The stored value does not carry the marker's expiry: it was written after
the marker, with
another TTL (`putTTL`, `keepAlive`) or none (a plain `put`), so it is kept
and the stale marker
goes.
- Right before the delete, the key no longer holds the bytes the cleanup
read: a renewal landed
since, so the value is kept and the stale marker goes. The bytes include
the expiry, so this
catches a renewal of the identical string.
- Otherwise the key and the marker are deleted.
The check and the delete are two store operations, so a renewal that lands
in between, after the
cleanup compared the bytes and before its two removes, is still deleted.
That is the window the
`txn` of #3376 closes.
## Verifying these changes
- [ ] Trivial rework / code cleanup without any test coverage. (No Need)
- [ ] Already covered by existing tests, such as *(please modify tests
here)*.
- [x] Need tests and can be verified as follows:
- `KvServiceTest` (`hg-pd-test`, `pd-core-test` suite), six new tests:
`testTtlCleanupDeletesTheExpiredKey` is the issue's measurement, it
fails on master's parse
and passes here; `testTtlCleanupKeepsAValueRenewedBeforeItsRead`,
`testTtlCleanupKeepsAValueRenewedAfterItsRead`,
`testTtlCleanupKeepsALeaseAKeepAliveRenewedAfterItsRead` and
`testTtlCleanupKeepsAPlainPutOverATtlValue` are the interleavings;
`testTtlCleanupDropsTheMarkerOfARemovedValue` is the absent value.
- Negative controls, the class run once with one guard removed (local,
JDK 17, 2026-10-20):
master's parse fails `testTtlCleanupDeletesTheExpiredKey`; without the
expiry check,
`testTtlCleanupKeepsAPlainPutOverATtlValue` and
`testTtlCleanupKeepsAValueRenewedBeforeItsRead`
fail; without the byte compare,
`testTtlCleanupKeepsAValueRenewedAfterItsRead` and
`testTtlCleanupKeepsALeaseAKeepAliveRenewedAfterItsRead` fail. All
eight pass with both.
- Suites on Pune101 (JDK 17, 2026-10-20, under the shared lock) at
75468dbdb: package and
`editorconfig:check` clean, pd-common 83/0/0/0, pd-core 196/0/0/2 (190
on master, the 6
new tests), pd-client 83/0/0/0 against a packaged PD (tests, failures,
errors, skipped).
## Does this PR potentially affect the following parts?
- [ ] Dependencies ([add/update
license](https://hugegraph.apache.org/docs/contribution-guidelines/contribute/#321-check-licenses)
info &
[regenerate_known_dependencies.sh](../install-dist/scripts/dependency/regenerate_known_dependencies.sh))
- [ ] Modify configurations
- [ ] The public API
- [x] Other affects (typed here): keys written with a TTL (`putTTL`, PD
locks, every
`KvService.put(key, value, ttl)` caller) now leave the store within a
second of their expiry,
as they were meant to. Before, an expired value stayed until a read found
it; no read served it
as live either way.
- [ ] Nope
## Documentation Status
Select one option and provide the documentation location when applicable.
- [ ] `Doc - TODO`: required documentation is pending; complete it before
merging.
- [ ] `Doc - Done`: documentation is included here or linked below.
- [x] `Doc - No Need`: no user-visible documentation is affected.
Documentation files in this PR or paired hugegraph-doc PR:
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
