anton-vinogradov commented on code in PR #13447:
URL: https://github.com/apache/ignite/pull/13447#discussion_r3752557739


##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheEntryInfo.java:
##########
@@ -64,58 +68,70 @@ public class GridCacheEntryInfo implements 
SelfMarshallingMessage, CacheIdAware
     /** Deleted flag. */
     private boolean deleted;
 
-    /** {@inheritDoc} */
-    @Override public int cacheId() {
-        return cacheId;
-    }
-
     /**
-     * @param cacheId Cache ID.
+     * Empty constructor for serialization purposes.
+     * see {@link #expireTimeDelta}.
      */
-    public void cacheId(int cacheId) {
+    public GridCacheEntryInfo() {
+        initTime = U.currentTimeMillis();
+    }
+
+    /** */
+    public GridCacheEntryInfo(int cacheId, KeyCacheObject key, @Nullable 
CacheObject val, GridCacheVersion ver, long expireTime, long ttl) {
+        assert expireTime >= 0;
+
+        if (expireTime != 0) {
+            initTime = U.currentTimeMillis();
+
+            expireTimeDelta = expireTime - initTime;
+
+            // In theory, here we can get the thread paused causing a negative 
delta value. Possible negative values

Review Comment:
   Wrong definition, see my comment about code relocation.
   GridCacheMapEntry.info() can now supply an obsolete expireTime (less than 
the current time)



##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheEntryInfo.java:
##########
@@ -19,40 +19,44 @@
 
 import org.apache.ignite.IgniteCheckedException;
 import org.apache.ignite.internal.Order;
-import org.apache.ignite.internal.SelfMarshallingMessage;
 import org.apache.ignite.internal.processors.cache.version.GridCacheVersion;
 import org.apache.ignite.internal.util.tostring.GridToStringInclude;
 import org.apache.ignite.internal.util.typedef.internal.S;
 import org.apache.ignite.internal.util.typedef.internal.U;
 import org.apache.ignite.plugin.extensions.communication.CacheIdAware;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.jetbrains.annotations.Nullable;
 
 /**
  * Entry information that gets passed over wire.
  */
-public class GridCacheEntryInfo implements SelfMarshallingMessage, 
CacheIdAware {
+public class GridCacheEntryInfo implements CacheIdAware, Message {
     /** */
     private static final int SIZE_OVERHEAD = 3 * 8 /* reference */ + 4 /* int 
*/ + 2 * 8 /* long */ + 32 /* version */;
 
     /** Cache key. */
     @Order(0)
     @GridToStringInclude
-    KeyCacheObject key;
+    @Nullable KeyCacheObject key;
 
     /** Cache ID. */
     @Order(1)
     int cacheId;
 
     /** Cache value. */
     @Order(2)
-    CacheObject val;
+    @Nullable CacheObject val;
 
     /** Time to live. */
     @Order(3)
     long ttl;
 
-    /** Expiration time. */
+    /** Base time to calculate {@link #expireTime()}. */
+    long initTime;

Review Comment:
   private?



##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheEntryInfo.java:
##########
@@ -64,58 +68,70 @@ public class GridCacheEntryInfo implements 
SelfMarshallingMessage, CacheIdAware
     /** Deleted flag. */
     private boolean deleted;
 
-    /** {@inheritDoc} */
-    @Override public int cacheId() {
-        return cacheId;
-    }
-
     /**
-     * @param cacheId Cache ID.
+     * Empty constructor for serialization purposes.
+     * see {@link #expireTimeDelta}.

Review Comment:
   Not a clear see. See for what?



##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheEntryInfo.java:
##########
@@ -190,30 +192,6 @@ public int marshalledSize(CacheObjectContext ctx) throws 
IgniteCheckedException
         return SIZE_OVERHEAD + size;
     }
 
-    // TODO IGNITE-28920: the rebase still runs inside the message; move it to 
the code filling and reading the entry.
-    /** {@inheritDoc} */
-    @Override public void selfMarshal() {
-        if (expireTime == 0)
-            expireTime = -1;
-        else {
-            expireTime -= U.currentTimeMillis();
-
-            if (expireTime < 0)
-                expireTime = 0;
-        }
-    }
-
-    /** {@inheritDoc} */
-    @Override public void selfUnmarshal() {
-        long remaining = expireTime;
-
-        expireTime = remaining < 0 ? 0 : U.currentTimeMillis() + remaining;
-
-        // Account for overflow.
-        if (expireTime < 0)
-            expireTime = 0;
-    }
-
     /** {@inheritDoc} */
     @Override public String toString() {

Review Comment:
   Missed new fields



##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheEntryInfo.java:
##########
@@ -64,58 +68,70 @@ public class GridCacheEntryInfo implements 
SelfMarshallingMessage, CacheIdAware
     /** Deleted flag. */
     private boolean deleted;
 
-    /** {@inheritDoc} */
-    @Override public int cacheId() {
-        return cacheId;
-    }
-
     /**
-     * @param cacheId Cache ID.
+     * Empty constructor for serialization purposes.
+     * see {@link #expireTimeDelta}.
      */
-    public void cacheId(int cacheId) {
+    public GridCacheEntryInfo() {
+        initTime = U.currentTimeMillis();
+    }
+
+    /** */
+    public GridCacheEntryInfo(int cacheId, KeyCacheObject key, @Nullable 
CacheObject val, GridCacheVersion ver, long expireTime, long ttl) {
+        assert expireTime >= 0;
+
+        if (expireTime != 0) {
+            initTime = U.currentTimeMillis();
+
+            expireTimeDelta = expireTime - initTime;
+
+            // In theory, here we can get the thread paused causing a negative 
delta value. Possible negative values
+            // shouldn't be treated as disabled expiration. Correct behavior 
is expired timeout.
+            if (expireTimeDelta < 0)
+                expireTimeDelta = 0;
+        }
+
         this.cacheId = cacheId;
+        this.key = key;
+        this.val = val;
+        this.ver = ver;
+        this.ttl = ttl;
+    }
+
+    /** {@inheritDoc} */
+    @Override public int cacheId() {
+        return cacheId;
     }
 
     /**
      * @param key Entry key.
      */
-    public void key(KeyCacheObject key) {
+    public void key(@Nullable KeyCacheObject key) {
         this.key = key;
     }
 
     /**
      * @return Entry key.
      */
-    public KeyCacheObject key() {
+    @Nullable public KeyCacheObject key() {
         return key;
     }
 
     /**
      * @return Entry value.
      */
-    public CacheObject value() {
+    public @Nullable CacheObject value() {
         return val;
     }
 
     /**
-     * @param val Entry value.
-     */
-    public void value(CacheObject val) {
-        this.val = val;
-    }
-
-    /**
-     * @return Expire time.
+     * @return Expire time >= 0. 0 means no expiration is set.
      */
     public long expireTime() {
-        return expireTime;
-    }
+        assert initTime > 0;

Review Comment:
   Will it fail on caches without expiration?



-- 
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]

Reply via email to