Github user StefanRRichter commented on a diff in the pull request:

    https://github.com/apache/flink/pull/6361#discussion_r203372004
  
    --- Diff: 
flink-end-to-end-tests/flink-stream-state-ttl-test/src/main/java/org/apache/flink/streaming/tests/verify/TtlUpdateContext.java
 ---
    @@ -0,0 +1,89 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.flink.streaming.tests.verify;
    +
    +import javax.annotation.Nonnull;
    +
    +import java.io.Serializable;
    +
    +/** Contains relevant for state TTL update data. */
    +public class TtlUpdateContext<UV, GV> implements Serializable {
    +   private final int key;
    +
    +   @Nonnull
    +   private final String verifierId;
    +
    +   private final GV valueBeforeUpdate;
    +   private final UV update;
    +   private final GV updatedValue;
    +
    +   private final long timestamp;
    +
    +   public TtlUpdateContext(int key, @Nonnull String verifierId, GV 
valueBeforeUpdate, UV update, GV updatedValue) {
    +           this(key, verifierId, valueBeforeUpdate, update, updatedValue, 
System.currentTimeMillis());
    --- End diff --
    
    This is recording the timestamp with some imprecision from the actual time 
that the TTL state saw. On the test server, this time difference can be 
significant and lead to flaky tests, even with your hardcoded imprecision. 
Instead, we could record the timestamp `t1` before accessing the TTL, and 
timestamp `t2` after accessing the TTL. All state with ttl < `t1` must clearly 
be expired, all with ttl > `t2` must clearly be there, and cases that fall 
between `t1` and `t2` could be ignored because we cannot clearly decide if they 
saw a timestamp before or after their expiration when accessed.


---

Reply via email to