IssueTracker

Hi, thanks for reaching out to us and reporting this issue! Will share this to the internal teams and open a report on your behalf. Will reach back with updates as soon as I have one.

Thanks!

The problem I see with a suspend refreshVersions() that simply notifies observers as usual is that it will cause a database transaction. Essentially on every page load all DB operation will get blocked, then I imagine a user flinging through a list, starving DB operations because on every load we need to force invalidation refresh. Secondly, imagine a long-transaction running for some other reason, an unrelated write, or a complicated read, it will delay the loading of the page because we are waiting to start a transaction to check for invalidation.

I also don't see how a suspend version is better than the sync, the load is already in an IO Dispatcher in some random IO thread from Kotlin, so its not really blocking Paging's ability to discard the source due to invalidation. There is little to no benefit to switching to a Room WRITE thread, other than serialization of transactions, which will just further delay things. Note that I am super-assuming here that Paging behaves well and discards a returned page if invalidate() has been called before load() is complete, which is what will happen with refreshVersionsSync() since by the time the method returns the observer in the PagingSource has been called.

Numero 3 is basically just:

fun isTableInvalidated(name: String): Boolean {
  val tableId = tableIdLookup[name] ?: false
  val stmnt = db.compileStatement("SELECT invalidate FROM room_table_modification_log WHERE table_id = ?)
  smnt.bindLong(0, tableId)
  return smnt.simpleQueryForLong() == 1
}

That is it, no transaction. Then the paging source can delay as much as they want or call invalidate() itself right there after the check, assuming that is allowed.

The part i don't fully understand is that refreshVersionsAsync wont obtain a transaction unless it has to:

https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:room/room-runtime/src/main/java/androidx/room/InvalidationTracker.java;l=503?q=InvalidationTracker

    public void refreshVersionsSync() {
        if (mAutoCloser != null) {
            // This increment call must be matched with a corresponding call in mRefreshRunnable.
            mAutoCloser.incrementCountAndEnsureDbIsOpen();
        }
        syncTriggers();
        mRefreshRunnable.run();
    }

    void syncTriggers(SupportSQLiteDatabase database) {
        if (database.inTransaction()) {
            return;
        }
        try {
            
            while (true) {
                Lock closeLock = mDatabase.getCloseLock();
                closeLock.lock();
                try {
                    final int[] tablesToSync = mObservedTableTracker.getTablesToSync();
                    if (tablesToSync == null) {
                        return;
                    }

@VisibleForTesting
    Runnable mRefreshRunnable = new Runnable() {
        @Override
        public void run() {
            final Lock closeLock = mDatabase.getCloseLock();
            Set<Integer> invalidatedTableIds = null;
            closeLock.lock();
            try {

                if (!ensureInitialization()) {
                    return;
                }

                if (!mPendingRefresh.compareAndSet(true, false)) {
                    // no pending refresh
                    return;

So if no endTransaction calls happened, as far as i can see, this won't create a transaction but will obtain a bunch of locks.

But obvioulsy, there are cases where transactions are used for reads and if paging does it itself (e.g. paging relational data), it might get into a transaction in every case but i don't know how we can even avoid it.

True that refreshVersionsSync() might not even check the DB or start a transaction if there are no pending refresh, but we have to solve for the worst case too right? It starts a transaction after any transaction is done, worst yet any WRITE operation (including non-observing tables) will cause refresh to do a transaction, so I really think it is just best to avoid going through path and having a more direct check for Paging, even if it has some false-positives.

An example of a false positive, would be: Do the LIMIT / OFFSET query where data is valid, then a WRITE happens before we check for invalid. This will mean we have a valid page to return but opted for invalidating the paging source (throwing InvalidDataException), this is OK.

yea false positives is fine. i'm not fully convinced that a new API is necessary just to check invalidation without transaction to be honest, but keeping an open mind about it.

We should write a design doc for this.

Hello, i’m facing a problem on some devices where i have a paging source with multiple tables query (relationships join) when the user clicks on an item from the list and navigates to the details page the details page refreshes the “progress” for the entity which is one of the paging source relationships which in turn invalidates the paging source, the details page have a list of items that also fetches a progress, when the user navigates back the list which is a paging source takes seconds to display the pages I noticed two things:

1. when the loaded pages are less the fastest the pages display

2. when i remove the “progress” relation from the paging source query the issue is completely gone and the list displayed quickly

I hope that this issue gets resolved soon, Thanks for your efforts.

Hello please find the attached sample project