orientechnologies / orientdb

OrientDB is the most versatile DBMS supporting Graph, Document, Reactive, Full-Text and Geospatial models in one Multi-Model product. OrientDB can run distributed (Multi-Master), supports SQL, ACID Transactions, Full-Text indexing and Reactive Queries.
https://orientdb.dev
Apache License 2.0
4.75k stars 871 forks source link

Caching and referenced records #6204

Closed kerner1000 closed 8 years ago

kerner1000 commented 8 years ago

Hi,

I experience some difficulties with the caching mechanism.

I have a document db and documents store references to other documents.

If I modify these references without altering the document itself, I can only see these changes after clearing the db's cache.

db.getLocalCache().clear()

see #6080.

How can I trigger a reload of documents that contain modified references without invalidating the whole cache?

Many thanks!

andrii0lomakin commented 8 years ago

If I modify these references without altering the document itself

What do you mean, could you provide example of code ?

kerner1000 commented 8 years ago

I have DocumentA and DocumentB

A has a one2ManyRelationship to B B has a many2OneRelationship to A

When I load more data into the db, I update A but not B.

When I display the db in my application, I query for B, which holds a reference to A.

If B contains a reference to any A, I display A, if not, I display B.

References are stored as Document IDs, so I assume the document with this ID is not checked for changes as well but just loaded from the cache.

public static void establishOne2ManyRelationship(ODocument documentFirst, String classNameFirst, String fieldNameFirst, ODocument documentSecond, String classNameSecond, String fieldNameSecond) {

        // left side
        addToReferenceCollection(documentFirst, classNameFirst, fieldNameFirst, documentSecond);
        // right side
        setReference(documentSecond, classNameSecond, fieldNameSecond, documentFirst);
    }
public static void establishMany2OneRelationship(ODocument documentFirst, String classNameFirst, String fieldNameFirst, ODocument documentSecond, String classNameSecond, String fieldNameSecond) {

        // left side
        setReference(documentFirst, classNameFirst, fieldNameFirst, documentSecond);
        // right side
        addToReferenceCollection(documentSecond, classNameSecond, fieldNameSecond, documentFirst);
    }
private static void addToReferenceCollection(ODocument document, String className, String fieldName, ODocument value) {

        if(isValid(document, className)) {
            Collection<ODocument> refCollection = new HashSet<>(getReferenceCollection(document, className, fieldName));
            refCollection.add(value);
            setReferenceCollection(document, className, fieldName, refCollection);
        } else
            throw new IllegalArgumentException("For " + document);
    }
public static Collection<ODocument> getReferenceCollection(ODocument document, String className, String fieldName) {

        if(isValid(document, className)) {
            try {
                Collection<OIdentifiable> id = document.field(fieldName, OType.LINKSET);
                if(id == null) {
                    return Collections.emptySet();
                }
                Collection<ODocument> result = new HashSet<>();
                for(OIdentifiable i : id) {
                    if(i != null) {
                        ODocument r = i.getRecord();
                        if(r != null)
                            result.add(r);
                    }
                }
                return result;
            } catch(Exception e) {
                logger.error(e.getLocalizedMessage(), e);
                return Collections.emptySet();
            }
        }
        throw new IllegalArgumentException("For " + document);
    }
private static void setReferenceCollection(ODocument document, String className, String fieldName, Collection<ODocument> value) {

        if(isValid(document, className)) {
            document = save(document.field(fieldName, value, OType.LINKSET));
        } else
            throw new IllegalArgumentException("For " + document);
    }
andrii0lomakin commented 8 years ago

@kerner1000 I see that you use static methods, do you share documents and database instances across multiple threads ?

kerner1000 commented 8 years ago

No, I already learned to work always with an instance of OPartitionedDatabasePool

The general pattern I use looks like this:

public class LibraryPeakDocuments {

    private final static Logger logger = Logger.getLogger(LibraryPeakDocuments.class);
    /**
     * The identifier for this database class
     */
    public final static String CLASS_NAME = "CHROMIDENT_LibraryPeakDocument";
    /**
     * A list of reference peaks in the database which are associated to this library peak
     */
    // One2Many relationship field
    public final static String FIELD_REFERENCE_PEAKS = "FIELD_ReferencePeaks";
    // One2One relationship field
    public final static String FIELD_MASS_SPECTRUM = "FIELD_MassSpectrum";
    private final static String NA_STRING = "";
    private static final Double NA_DOUBLE = -1.0;
    private static final Integer NA_INTEGER = -1;
    private final OPartitionedDatabasePool pool;
    private ODatabaseDocument db;

    public LibraryPeakDocuments(ODatabaseDocument db) {
        this.pool = null;
        this.db = db;
    }

    public LibraryPeakDocuments(OPartitionedDatabasePool pool) {
        this.pool = pool;
    }

    public void addReferencePeak(ODocument libraryPeakDocument, ODocument referencePeakDocument) {

        try {
            if(pool != null) {
                db = pool.acquire();
            }
            Documents.establishOne2ManyRelationship(libraryPeakDocument, CLASS_NAME, FIELD_REFERENCE_PEAKS, referencePeakDocument, ReferencePeakDocuments.CLASS_NAME, ReferencePeakDocuments.FIELD_LIBRARY_PEAK);
            updateMergedMassSpectrum(libraryPeakDocument, referencePeakDocument);
        } finally {
            if(pool != null) {
                db.close();
            }
        }
    }

    public void clearReferencePeaks(ODocument libraryPeakDocument) {

        try {
            if(pool != null) {
                db = pool.acquire();
            }
            Documents.releaseOne2ManyRelationship(libraryPeakDocument, CLASS_NAME, FIELD_REFERENCE_PEAKS, getReferencePeaks(libraryPeakDocument), ReferencePeakDocuments.CLASS_NAME, ReferencePeakDocuments.FIELD_LIBRARY_PEAK);
        } finally {
            if(pool != null) {
                db.close();
            }
        }
    }

    public void delete(ODocument libraryPeakDocument) {

        try {
            if(pool != null) {
                db = pool.acquire();
            }
            Documents.releaseOne2ManyRelationship(libraryPeakDocument, CLASS_NAME, FIELD_REFERENCE_PEAKS, getReferencePeaks(libraryPeakDocument), ReferencePeakDocuments.CLASS_NAME, ReferencePeakDocuments.FIELD_LIBRARY_PEAK);
            ODocument doc = getMassSpectrumDocument(libraryPeakDocument);
            Documents.delete(doc);
            Documents.delete(libraryPeakDocument);
        } finally {
            if(pool != null) {
                db.close();
            }
        }
    }

    public IScanMSD getMassSpectrum(ODocument peakDocument) {

        try {
            if(pool != null) {
                db = pool.acquire();
            }
            ODocument ref = getMassSpectrumDocument(peakDocument);
            if(ref == null) {
                return null;
            }
            return MassSpectrumDocuments.getMassSpectrum(ref);
        } finally {
            if(pool != null) {
                db.close();
            }
        }
    }

    public ODocument getMassSpectrumDocument(ODocument peakDocument) {

        try {
            if(pool != null) {
                db = pool.acquire();
            }
            return Documents.getReference(peakDocument, CLASS_NAME, FIELD_MASS_SPECTRUM);
        } finally {
            if(pool != null) {
                db.close();
            }
        }
    }

    public Collection<ODocument> getReferencePeaks(ODocument libraryPeakDocument) {

        try {
            if(pool != null) {
                db = pool.acquire();
            }
            return Documents.getReferenceCollection(libraryPeakDocument, CLASS_NAME, FIELD_REFERENCE_PEAKS);
        } finally {
            if(pool != null) {
                db.close();
            }
        }
    }

    public boolean isValid(ODocument document) {

        try {
            if(pool != null) {
                db = pool.acquire();
            }
            return Documents.isValid(document, CLASS_NAME);
        } finally {
            if(pool != null) {
                db.close();
            }
        }
    }

    public void removeReferencePeak(ODocument libraryPeakDocument, ODocument peakDocument) {

        try {
            if(pool != null) {
                db = pool.acquire();
            }
            Documents.releaseOne2ManyRelationship(libraryPeakDocument, CLASS_NAME, FIELD_REFERENCE_PEAKS, peakDocument, ReferencePeakDocuments.CLASS_NAME, ReferencePeakDocuments.FIELD_LIBRARY_PEAK);
            if(getReferencePeaks(libraryPeakDocument).isEmpty()) {
                delete(libraryPeakDocument);
            } else {
                updateMergedMassSpectrum(libraryPeakDocument);
            }
        } finally {
            if(pool != null) {
                db.close();
            }
        }
    }

    protected void setMassSpectrumDocument(ODocument peakDocument, ODocument massSpectrumDocument) {

        Documents.setReference(peakDocument, CLASS_NAME, FIELD_MASS_SPECTRUM, massSpectrumDocument);
    }

}
andrii0lomakin commented 8 years ago

@kerner1000 that is incorrect code. You can not share documents between threads, but you may pass rid. so you code should look like following: https://gist.github.com/laa/f60d4315186fb988694309fe63ed7d7c Does it work now ?

kerner1000 commented 8 years ago

@laa , many thanks for this example! I will refactor the code according to your example and check, if I can load the DB without clearing the cache.

andrii0lomakin commented 8 years ago

@kerner1000 any updates ?

kerner1000 commented 8 years ago

Hi @laa I refactored to code to use ORID instead of ODocument. But I experience some difficulties establishing document relationships/ links as I described here I will do some more testing to see if it works without touching the cache. Many thanks for your help!

andrii0lomakin commented 8 years ago

Hi @kerner1000 , any updates ?

kerner1000 commented 8 years ago

Hi @laa , I refactored the code as shown in your example but I am afraid we are still confronted with this issue. I was thinking a lot about this and I guess the problem is somewhere else. That is because we re-load the complete database after each modification, using

/**
     * Retrieves all documents for given class name.
     * 
     * @param className
     *            for which all documents should be retrieved
     * @return {@link Iterator} over all documents for given class name
     */
    public Iterator getAll(String className) {
        return executeOnDB(db -> {
            try {
                return db.browseClass(className);
            } catch(Exception e) {
                // exception also thrown when no class for this class name could be found (instead
                // of returning an empty iterator
                return new ArrayList().iterator();
            }
        });
    }

Since we re-load everything and update the complete table, I think it does not really matter if we pass around ORID, ODocument or OIdentifiable. The complete thing is anyways re-build from a fresh db query.

What do you think?

Many thanks for helping! Best, Alex

andrii0lomakin commented 8 years ago

@kerner1000 what do you do with the iterator? You see cache is cleared when we close connection, so there is no need to reload cache.

kerner1000 commented 8 years ago

Hm, this is strange. I refactored already everything to use a ConnectionPool, so the cache should be cleared automatically. This is what we do with the loaded documents (as mentioned, I had to re-added the 'clear cache' command):

db.acquire().getLocalCache().invalidate();
int lpCnt = 0, pCnt = 0;
Iterator documentIterator = new DatabasesOrientDBChromIdent(db).getAll(LibraryPeakDocuments.CLASS_NAME);
eventListLibraryPeaks.clear();
while(documentIterator.hasNext()) {
    eventListLibraryPeaks.add(new DocumentWrapper3(db, documentIterator.next().getIdentity()));
    lpCnt++;
}
documentIterator = new DatabasesOrientDBChromIdent(db).getAll(ReferencePeakDocuments.CLASS_NAME);
eventListReferencePeaks1.clear();
while(documentIterator.hasNext()) {
    ODocument document = documentIterator.next();
    if(!ReferencePeakDocuments.getLibraryPeak(document).isPresent()) {
        eventListReferencePeaks1.add(new DocumentWrapper3(db, document.getIdentity()));
        pCnt++;
    }
}
logger.debug("Displaying " + lpCnt + " library peaks, " + pCnt + " reference peaks");
natTableLibraryPeaks.refresh();
natTableReferencePeaks0.refresh();
natTableReferencePeaks1.refresh();

Here is the class that is called by the GUI to actually display the values:

https://gist.github.com/kerner1000/366d824564f5d7168ac9ff30ae23ce47 https://gist.github.com/kerner1000/85120ab861513bdc02aedc535fec3360

andrii0lomakin commented 8 years ago

@kerner1000 code seems ok. But if you still have an invalid cache at the beginning of operation probably it is not closed somewhere. Could you debug and check that open and close calls are matched everywhere ?

kerner1000 commented 8 years ago

Ok, I will try that! I will pull the sources and set a breakpoint in the ODatabaseDocument class.

kerner1000 commented 8 years ago

Hi, Im still debugging, I assume I close all connections. Is there a way to check for open connections in the Java API?

andrii0lomakin commented 8 years ago

@kerner1000 yes, the latest version of OrientDB has embedded check. But it is not released yet. So you should build from sources. It is simple mvn clean install -DskipTests

kerner1000 commented 8 years ago

@laa And how do I use it? Is there a method that returns all open connections?

andrii0lomakin commented 8 years ago

@kerner1000 no , it throws exception if you try to open database which is already opened from other thread.

kerner1000 commented 8 years ago

Hi, Im still facing the problem.

Right now I am using the following code to force-close a left-over database connection:

public static void dirtyDBClose() {
    int cnt = 0;
    ODatabaseDocument db = null;
    try {
        db = ODatabaseRecordThreadLocal.INSTANCE.get();
        while (db != null && !db.isClosed()) {
        db.close();
        cnt++;
        db = ODatabaseRecordThreadLocal.INSTANCE.get();
        }
    } catch (ODatabaseException e) {
        db = null;
    } finally {
        logger.warn("Finished dirty close after " + cnt + " attempts, db closed: " + (db == null || db.isClosed()));
    }
    }

I assume the problem still exists because I pass instances of ORID and OPartitionedDatabasePool to other threads, which eventually load a document using this ORID and the pool.

I was wondering, what happens with the thread-local db instance after a thread finishes? Will it be automatically closed?

andrii0lomakin commented 8 years ago

Hi, @kerner1000, Java does not have means to track "thread becomes dead" events. So we can not close a database in such case. If you send the code to reproduce an issue, we will either find the reason of issue or fix it. But without code, it is hard to say something about the reason. You are the single user, who has given problem. That is strange. You may send code any size it does not matter.

andrii0lomakin commented 8 years ago

@kerner1000 is it possible to send code to reproduce issue ?

andrii0lomakin commented 8 years ago

Hi @kerner1000 , I will really appreciate if you provide code which I may debug and identify reason.

kerner1000 commented 8 years ago

HI @laa I did not forget about this. I am currently trying to pack together a minimal example to reproduce the problem. So far I didn't have success. I do not want you to set up a complete development environment including the complete application. This would just be too much and would take too much time. I will keep you definitely updated, thanks a lot for your help, I appreciate it! Best Alex

andrii0lomakin commented 8 years ago

Close issue because of no reply. If you reproduce given issue and have information which is requested in issue feel free to reopen it.