NullPointerExceptions during stress testing with ORecordCacheSoftRefs

[agarciadom]

OrientDB Version, operating system, or hardware.

2.2.8

Operating System

Linux (Ubuntu 16.04)

Expected behavior and actual behavior

During a stress test with 8+ concurrent connections (2-core machine with hyperthreading) a to an embedded local database with 5M+ nodes while using ORecordCacheSoftRefs, Orient will sometimes fail to retrieve records due to NullPointerExceptions originated in the evictStaleEntries method of the underlying OSoftRefsHashMap cache.

In some heavily starved situations, reverseLookup.remove(sv) will return null:

  private void evictStaleEntries() {
    int evicted = 0;

    Reference<? extends V> sv;
    while ((sv = refQueue.poll()) != null) {
      hashCodes.remove(reverseLookup.remove(sv));
      evicted++;
    }

    if (evicted > 0)
      OLogManager.instance().debug(this, "Evicted %d items", evicted);
  }

Steps to reproduce the problem

• Create N*2 ODatabaseDocumentTx instances pointing to the same plocal:// embedded database.
• Run a long running query in all of them at once.
• Some of the initial queries will work, but once Java starts to reclaim memory, some of the queries will start to fail.

I am not sure why the reverseLookup.remove(sv) call might be returning null: this happens even if I use a simple ThreadLocal<ODatabaseDocumentTx> instead of one of Orient's pools (ensuring each thread has its own instance).

Proposed fix

Adding a simple null check to the method seems to work fine:

    private void evictStaleEntries() {
        int evicted = 0;

        Reference<? extends V> sv;
        while ((sv = refQueue.poll()) != null) {
            final K key = reverseLookup.remove(sv);
            if (key != null) {
                hashCodes.remove(key);
                evicted++;
            }
        }

        if (evicted > 0)
            OLogManager.instance().debug(this, "Evicted %d items", evicted);
    }

[tglman]

hi @bluezio,

Nice investigation, and as well the fix you suggest make sense, feel free to do a pull request about it, we will be happy to merge it.

In any case we are going to double check that part.

bye

[tglman]

manually merged, closing this