1

Context: There is an Infinispan (13.0.10) cache that uses a custom data type Quad<String, Long, Type, ValidityPeriod> as key. As far as I can tell all hashCode() and equals(Object o)-methods are either auto-generated or implemented correctly. Also the cache is used in Local Mode.

Question: When I try to call cache.get(Object key) with a Quad that I know the cache contains, I get null. cache.containsKey(Object key) also returns false.

How is this possible?

Clarification: I said that I know the cache contains the Quad key because I have done the following using the Eclipse IDE debugger:

cache.entrySet().stream().filter(e -> e.getKey().hashCode == Quad.of(a, b, c, d).hashCode()).collect(toList());
// this returns a non-empty list with my expected return
cache.entrySet().stream().filter(e -> e.getKey().equals(Quad.of(a, b, c, d))).collect(toList());
// this returns a non-empty list with my expected return

Per Radim Vansa's suggestion I have also tried the following:

cache.entrySet().stream().collect(Collectors.toMap(Entry::getKey, Entry::getValue, (o1,o2) -> o1, ConcurrentHashMap::new)).get(Quad.of(a, b, c, d));
// this returns my expected return

Further Context (if needed): I am working on an older project where I am supposed to update from Infinispan Version 10 to 13. I have successfully done so and also integrated ProtoStream API instead of using the old JBossMarshaller. This version change is important because the retrieval stopped working after the update.

There are multiple Caches in use, some are indexed using custom data types such as Pair<K, V>, Triple<L, M, R> and Quad<A, B, C, D> which are all generic. I have written some ProtoAdpaters for all of them and they work just fine.

I now have come across a problem, where there is an AdvancedCache that uses a Quad like this as key:

Quad<String, Long, Type, ValidityPeriod>

Quad overrides equals(Object o) and hashCode as follows:

public class Quad<A, B, C, D> {

    // other code omitted for brevity

    public boolean equals(final Object obj) {
        if (obj == this) {
            return true;
        }
        if (obj instanceof Quad<?, ?, ?, ?>) {
            final Quad<?, ?, ?, ?> other = (Quad<?, ?, ?, ?>) obj;
            return Objects.equals(getFirst(), other.getFirst())
                && Objects.equals(getSecond(), other.getSecond())
                && Objects.equals(getThird(), other.getThird())
                && Objects.equals(getFourth(), other.getFourth());
        }
        return false;
    }

    public int hashCode() {
        return Objects.hashCode(getFirst()) 
                ^ Objects.hashCode(getSecond()) 
                ^ Objects.hashCode(getThird()) 
                ^ Objects.hashCode(getFourth());
    }
}

For reference Type is structured along the lines of:

public class Type implements Serializable {
    private int fieldA;
    private int fieldB;
    private String fieldC;

    // hashCode and equals are auto-generated
    // constructor, getters and setters omitted for brevity
}

ValidityPeriod is something like this:

public class ValidityPeriod implements Serializable {
    private LocalDate validFrom;
    private LocalDate invalidFrom;

    // hashCode and equals are auto-generated
    // constructor, getters and setters omitted for brevity
}

The marshaller for LocalDate uses the following adapter:

@ProtoAdapter(LocalDate.class)
public class LocalDateAdapter {
    
    @ProtoFactory
    LocalDate create(int year, short month, short day) {
        return LocalDate.of(year, month, month);
    }
    
    @ProtoField(number = 1, required = true)
    int getYear(LocalDate localDate) {
        return localDate.getYear();
    }
    
    @ProtoField(number = 2, required = true)
    short getMonth(LocalDate localDate) {
        return (short) localDate.getMonth().getValue();
    }
    
    @ProtoField(number = 3, required = true)
    short getDay(LocalDate localDate) {
        return (short) localDate.getDayOfMonth();
    }
}

I have tried to use the debugger to understand the internal workings of Infinispan but I don't seem to be able to pin down a concrete line producing this error. As far as I know it has something to do with CacheImpl.get(Object, long, InvocationContext).

Update: Ok, per Pruivo's suggestion I have tried to create a reprex. The strange thing however is that I have tried to copy every process that happens before the object is retrieved from the cache, but in the reprex I created it works. Funny thing however is the following I tried: I created two methods in ValidityPeriod that do the following (almost like Infinispan Transformers):

public String toFormString() {
    return String.format("%s§%s", validFrom, invalidFrom);
}
    
public static ValidityPeriod fromFormString(String form) {
    String[] split = form.split("§");
    return from(LocalDate.parse(split[0]),LocalDate.parse(split[1]));
}

Then I've changed the Quad to Quad<String, Long, Type, String> while constructing the key for the cache with strings from these methods instead of the ValidityPeriod itself. Strangely enough, this fixes my original problem. However, as this is a dirty fix, I am not content with keeping this solution permanent. There has to be something wrong with ValidityPeriod is my guess.

I still am confused as to why the cache returns something different than it contains, so I will still leave my original question open.

Thomas
  • 41
  • 8
  • So `cache.keySet().stream()...` works but at the same time `cache.get(Quad.of(a, b, c, d))` doesn't? – Thomas Aug 02 '22 at 09:23
  • @Thomas Yes, that is where I am confused – Thomas Aug 02 '22 at 09:25
  • 1
    Could you try to retrieve the object using stream(), put it into a ConcurrentHashMap and retrieve it from there? I smell a subtle bug in hashCode()/equals() anyway. I guess the Quad is a POJO, or are there any transient/volatile fields? – Radim Vansa Aug 03 '22 at 07:17
  • @Radim Vansa, thanks for your idea. I will try that and update my question. None of the fields of Quad are transient/volatile, same goes for the elements' classes. – Thomas Aug 03 '22 at 08:35
  • @RadimVansa I have tried it and sadly the correct Object gets returned. I've updated the question with the call I made (under clarification). – Thomas Aug 03 '22 at 14:37
  • Can you create a reproducer? It is probably easier than just asking for stuff to test. – pruivo Aug 03 '22 at 16:31
  • @pruivo I will try to do that. As the project I'm working in is monolithic (and not exactly small) it's a little difficult but I'll try my best – Thomas Aug 04 '22 at 09:10
  • @Pruivo As you can see in my update, I wasn't able to reproduce the problem outside the project. I don't know why. But your idea gave me a direction which lead me to the more precise error I stated under "Update". Thanks :) – Thomas Aug 05 '22 at 11:23
  • 1
    You said you are using `ProtoStream`. Can you share the marshaller for `LocalDate`? Maybe some field is missing... – pruivo Aug 07 '22 at 15:07
  • @pruivo just as I posted the adapter I saw it. The field `day` is not correctly set in `create`. Oh my god! Nasty typo! Oh man. Thanks for the direction! I will leave this here as info for others to triple-check their protoadapters. – Thomas Aug 08 '22 at 09:03
  • @Pruivo I am still wondering how it's possible that `hashCode()` and `equals()` were working though. My guess is that these methods are called directly on the pojo-cache and `get()`, `containsKey()` are not. As Infinispan implements the Map-Interface this kind of behaviour seems confusing. – Thomas Aug 08 '22 at 09:13
  • 1
    Infinispan uses `hashCode()` to determine the segment (and owners) of a key. Internally, the data is stored in a `ConcurrentHashMap`. My guess is, if the protostream adapter was incorrect, some nodes would store a "different version" of your key, and then `get()` and `containsKey()` would fail to match. We would need to debug Infinispan to find out the place where the mismatch happens. – pruivo Aug 08 '22 at 11:00
  • @Pruivo Thanks for the insight. I will keep that in mind. As for the debugging, actually I was using Infinispan only on my local machine (but in Replicated Mode). The cache gets written to a SoftFileStore though. I've also found the `ConcurrentHashMap` that's used inside the cache and it calls `get(key)` with a key that is serialized. This call is made on a map which's keys and values are also serialized. Is this normal behaviour? – Thomas Aug 09 '22 at 11:16
  • by default, embedded stores POJO but can be configured to store serialized objects. Server stores the serialized objects by default. – pruivo Aug 09 '22 at 18:23
  • @Pruivo Thanks for all the usefull insights. I think I will summarize the solution for my problem in an answer and use the relevant parts of information you gave to do so. Feel free to edit if I get anything wrong. – Thomas Aug 10 '22 at 14:30

1 Answers1

2

After a careful deduction following Pruivo's suggestions, I could pin down a cause for this bug. It is the ProtoAdapter that is written for LocalDate in which an incorrect LocalDate-object is created upon deserialization.

Concrete fix: The concrete fix for this case is the following:

@ProtoAdapter(LocalDate.class)
public class LocalDateAdapter {
    
    @ProtoFactory
    LocalDate create(int year, short month, short day) {
        return LocalDate.of(year, month, day); // <= here was the error
    }
    
    @ProtoField(number = 1, required = true)
    int getYear(LocalDate localDate) {
        return localDate.getYear();
    }
    
    @ProtoField(number = 2, required = true)
    short getMonth(LocalDate localDate) {
        return (short) localDate.getMonth().getValue();
    }
    
    @ProtoField(number = 3, required = true)
    short getDay(LocalDate localDate) {
        return (short) localDate.getDayOfMonth();
    }
}

Explanation: As Pruivo stated, Infinispan uses hashCode() to determine a key's owner/segment. In the background Infinispan uses a ConcurrentHashMap for storing keys and values. This map contains serialized objects if the cache is configured for that.

Now to the problem. The ConcurrentHashMap contains the following method that is called to retrieve an object:

public V get(Object key) {
   Node<K,V>[] tab; Node<K,V> e, p; int n, eh; K ek;
   int h = spread(key.hashCode());
   if ((tab = table) != null && (n = tab.length) > 0 &&
      (e = tabAt(tab, (n - 1) & h)) != null) { // <= line 5
         // this code here is not reached
         if ((eh = e.hash) == h) {
                if ((ek = e.key) == key || (ek != null && key.equals(ek)))
                    return e.val;
            }
            else if (eh < 0)
                return (p = e.find(h, key)) != null ? p.val : null;
            while ((e = e.next) != null) {
                if (e.hash == h &&
                    ((ek = e.key) == key || (ek != null && key.equals(ek))))
                    return e.val;
         }
      }
   }
   return null;
}

As you can see in line 5, there is a check for a method called tabAt(). This checks if the object reference at that adress of the map is null. If so then no further actions are taken and null will be returned. This includes that no check for hashCode() or equals() are done under this condition. This you can see in the lines that follow tabAt(). So this situation explains why cache.entrySet().stream()... could retrieve an object an get() couldn't.

Now my educated guess why the object's reference isn't found: My quess is that the original key is that of the correctly serialized object but after deserialization this key becomes corrupt as it is no longer the correct one. As modification of a key in a Map can produce unexpected behaviour, this is my theory. (Source for unexpected behaviour of Map when key is modified: https://stackoverflow.com/a/21601013/14945497)

Suggestion/Remark: A final remark I would like to make is that this error can be quite inconspicuous and lead to problems down the line because wrong objects are created upon deserialization. This leads to a lot of detective work when an error does occur. So, double-triple-check your definitions for all marshallers is my suggestion.

Thomas
  • 41
  • 8