rob-9 commented on code in PR #885:
URL: https://github.com/apache/flink-agents/pull/885#discussion_r3838041606
##########
runtime/src/main/java/org/apache/flink/agents/runtime/actionstate/KafkaActionStateStore.java:
##########
@@ -175,10 +181,13 @@ public ActionState get(Object key, long seqNum, Action
action, Event event) thro
// the requested seqNum
return stateSeqNum > seqNum;
}
- } catch (NumberFormatException e) {
+ } catch (IllegalArgumentException e) {
LOG.warn(
- "Failed to parse sequence number
from state key: {}",
- stateKey);
+ "Cannot parse state key: {}. The
entry cannot be "
+ + "considered for
divergence cleanup and will "
+ + "be retained.",
+ entry.getKey(),
Review Comment:
Demoted this to DEBUG and removed the throwable, so repeated cache misses no
longer produce WARN stack traces.
##########
runtime/src/test/java/org/apache/flink/agents/runtime/actionstate/KafkaActionStateStoreTest.java:
##########
@@ -142,6 +165,16 @@ void testGetActionStateWithDiverge() throws Exception {
assertNull(actionStateStore.get(TEST_KEY, 4L, testAction, testEvent));
}
+ @Test
+ void testGetRetainsUnparseableKey() throws Exception {
+ String flinkKey = "user_123";
+ String stateKey = ActionStateUtil.generateKey(flinkKey, 1L,
testAction, testEvent);
+ actionStates.put(stateKey, testActionState);
+
+ assertThat(actionStateStore.get(flinkKey, 2L, testAction,
testEvent)).isNull();
Review Comment:
you're right. I inverted the sequence numbers so the test now fails if the
key parses successfully and gets evicted.
##########
runtime/src/test/java/org/apache/flink/agents/runtime/actionstate/FlussActionStateStoreTest.java:
##########
@@ -121,6 +121,17 @@ void testGetTriggersDivergenceCleanup() throws Exception {
assertThat(store.get(TEST_KEY, 3L, testAction, testEvent)).isNull();
}
+ @Test
+ void testPruneStateRetainsKeyContainingUnderscore() throws Exception {
+ String agentKey = "user_123";
+ String stateKey = ActionStateUtil.generateKey(agentKey, 1L,
testAction, testEvent);
+ actionStates.put(stateKey, testActionState);
+
+ store.pruneState(agentKey, 1L);
+
+ assertThat(actionStates).containsKey(stateKey);
Review Comment:
added the exact parsed-key check to Fluss, with tests covering both pruning
and lookup cleanup.
--
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]
