duongkame commented on PR #1126:
URL: https://github.com/apache/ratis/pull/1126#issuecomment-2258740445
> @duongkame , instead of reverting this, how about replacing the guava
Cache with a `WeakHashMap`?
>
> ```java
> diff --git
a/ratis-server-api/src/main/java/org/apache/ratis/server/protocol/TermIndex.java
b/ratis-server-api/src/main/java/org/apache/ratis/server/protocol/TermIndex.java
> index a8aa670613..e3b8475fe0 100644
> ---
a/ratis-server-api/src/main/java/org/apache/ratis/server/protocol/TermIndex.java
> +++
b/ratis-server-api/src/main/java/org/apache/ratis/server/protocol/TermIndex.java
> @@ -19,23 +19,23 @@ package org.apache.ratis.server.protocol;
>
> import org.apache.ratis.proto.RaftProtos.LogEntryProto;
> import org.apache.ratis.proto.RaftProtos.TermIndexProto;
> -import org.apache.ratis.thirdparty.com.google.common.cache.Cache;
> -import org.apache.ratis.thirdparty.com.google.common.cache.CacheBuilder;
>
> +import java.lang.ref.WeakReference;
> import java.util.Comparator;
> import java.util.Optional;
> -import java.util.concurrent.ExecutionException;
> -import java.util.concurrent.TimeUnit;
> +import java.util.WeakHashMap;
>
> /** The term and the log index defined in the Raft consensus algorithm. */
> public interface TermIndex extends Comparable<TermIndex> {
> class Util {
> /** An LRU Cache for {@link TermIndex} instances */
> - private static final Cache<TermIndex, TermIndex> CACHE =
CacheBuilder.newBuilder()
> - .maximumSize(1 << 16)
> - .expireAfterAccess(1, TimeUnit.MINUTES)
> - .build();
> + private static final WeakHashMap<TermIndex, WeakReference<TermIndex>>
CACHE = new WeakHashMap<>();
> +
> + private static synchronized TermIndex putIfAbsent(TermIndex
termIndex) {
> + return CACHE.computeIfAbsent(termIndex, WeakReference::new).get();
> + }
> }
> +
> TermIndex[] EMPTY_ARRAY = {};
>
> /** @return the term. */
> @@ -109,10 +109,6 @@ public interface TermIndex extends
Comparable<TermIndex> {
> return String.format("(t:%s, i:%s)", longToString(term),
longToString(index));
> }
> };
> - try {
> - return Util.CACHE.get(key, () -> key);
> - } catch (ExecutionException e) {
> - throw new IllegalStateException("Failed to valueOf(" + term + ", "
+ index + "), key=" + key, e);
> - }
> + return Util.putIfAbsent(key);
> }
> }
> \ No newline at end of file
> ```
I don't think we need any cache here. @szetszwo . You already create the
TermIndex instance and throw it away to favor the one in the cache. How does it
help?
--
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]
