bitflicker64 commented on code in PR #3382:
URL: https://github.com/apache/hugegraph/pull/3382#discussion_r4333004379
##########
hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/KvService.java:
##########
@@ -297,18 +298,65 @@ public void clearTTLData() {
byte[] ttlStartKey = getTTLStoreKey("", 0);
byte[] ttlEndKey = getTTLStoreKey("", System.currentTimeMillis());
List<KV> kvList = meta.scanRange(ttlStartKey, ttlEndKey);
+ // A marker is K@T@<time>@<key>: the key starts after the
delimiter that ends the time
+ int timeStart = (KV_PREFIX_DELIMITER + TTL_PREFIX +
KV_DELIMITER).length();
for (KV kv : kvList) {
String key = new String(kv.getKey());
- int index = key.indexOf(KV_DELIMITER, 2);
+ int index = key.indexOf(KV_DELIMITER, timeStart);
String delKey = key.substring(index + 1);
- delete(delKey);
- meta.remove(kv.getKey());
+ byte[] stored = meta.getOne(getStoreKey(delKey));
+ removeExpired(delKey,
key.substring(KV_PREFIX_DELIMITER.length()), stored);
}
} catch (Exception e) {
log.error("clear ttl data with error :", e);
}
}
+ /**
+ * Deletes a key whose TTL marker expired, and the marker, while the key
still holds the
+ * value the marker names. The value the cleanup read must carry the
marker's expiry, so a
+ * value written after the marker (with another TTL, or none) is kept and
only the stale
+ * marker goes. Right before the delete, the key must still hold the bytes
the cleanup read:
+ * a renewal writes another expiry, so its bytes differ even for the same
string. The check
+ * and the delete are separate store operations, so a renewal that lands
between them is
+ * still deleted; binding them in one raft entry needs the txn RPC of
#3376.
+ *
+ * @param key the key, without the K@ prefix
+ * @param marker the marker key, without the K@ prefix: T@<time>@<key>
+ * @param stored what the cleanup read under the key, null if it is absent
+ */
+ public void removeExpired(String key, String marker, byte[] stored) throws
PDException {
+ byte[] storeKey = getStoreKey(key);
+ byte[] markerKey = getStoreKey(marker);
+ if (stored == null) {
+ meta.remove(markerKey);
+ return;
+ }
+ V read;
+ try {
+ read = V.parseFrom(stored);
+ } catch (InvalidProtocolBufferException e) {
+ // Not a value KvService wrote: the plain delete of before
+ meta.remove(storeKey);
+ meta.remove(markerKey);
+ return;
+ }
+ int timeEnd = marker.indexOf(KV_DELIMITER, TTL_PREFIX.length() + 1);
+ long expireAt = Long.parseLong(marker.substring(TTL_PREFIX.length() +
1, timeEnd));
+ if (read.getTtl() != expireAt) {
+ // Written after the marker, with another TTL or none: not the
value it names
+ meta.remove(markerKey);
+ return;
+ }
+ if (!Arrays.equals(meta.getOne(storeKey), stored)) {
+ // The key changed since the read: keep it, drop the marker that
named the old value
+ meta.remove(markerKey);
+ return;
+ }
+ meta.remove(storeKey);
Review Comment:
Fixed in 37ca25508. Reproduced first: `put(key, value, 10_000_000_000_000L)`
then `clearTTLData()` deleted the value at 75468dbdb. `removeExpired` now
leaves a marker whose time has not passed alone, with its value, using
`getValue`'s test (`expireAt >= now`), so the byte-wise scan picking it early
is harmless. Test: `testTtlCleanupKeepsAValueThatHasNotExpired` (value and
marker kept; fails with the check removed). The three interleaving tests now
pass a marker that is due, as the scan does; the keepAlive one had a 100 s
lease and would have stopped at the new check.
--
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]
