Repository: libsql/libsql
File: libsql-sqlite3/src/wal.c (C)

Code before:
    p = 0;
  }
  *pp = p;
  return rc;
}

static int walIteratorRevInit(Wal *pWal, u32 nBackfill, struct WalIteratorRev *p, u32 mxSafeFrame, int getMissingFrame){
    WalIterator *pIter;
    u32 *frames;
    u32 iFrame, iPageno;
    int rc;

    frames = (u32*)sqlite3MallocZero((pWal->hdr.mxFrame + 1) * sizeof(u32));
    if (!frames) return SQLITE_NOMEM_BKPT;
    rc = walIteratorInit(pWal, nBackfill, &pIter);
    if (rc || !pIter) {
        sqlite3_free(frames);
        return rc;
    }

    while (walIteratorNext(pIter, &iPageno, &iFrame) == 0) {
        /*
         * If we get a page with a frame_no greater than mxSafeFrame, and getMissingFrame is true,
         * then we replace it with the latest page with frame_no <= mxSafeFrame.
         */
        if (iFrame > mxSafeFrame && getMissingFrame) {
           rc = walFindFrame(pWal, iPageno, mxSafeFrame, &iFrame);
           if( rc!=SQLITE_OK ) break;
           if (iFrame == 0) {
               continue;
           }
        }
        frames[iFrame] = iPageno;
    }
    walIteratorFree(pIter);
    if (rc != 0) {
        sqlite3_free(frames);
        return rc;
    }

    p->current = pWal->hdr.mxFrame;
    p->frames = frames;

    return SQLITE_OK;
}

static void walIteratorRevFree(struct WalIteratorRev *p) {
    p->current = 0;
    sqlite3_free(p->frames);
}


Review comment (bug):
so do I understand it correctly that `walIteratorRevInit` is actually linear, and it iterates the whole WAL structure, remembering all the frame positions in memory, so that we can iterate it backwards later?

If it's the case, it sounds like a memory usage and performance regression, and even if we want to bet on it for libsql-server, it should be opt-in, it could kill lots of valid use cases where the WAL is large and there's not that much memory available -- esp. if somebody runs SQLite with soft and hard memory limits set up. Any attempt at checkpoint will just return an allocation failure.

Code after:
    }
  }
  if( rc!=SQLITE_OK ){
    SEH_FREE_ON_ERROR(p, 0);
    walIteratorFree(p);
    p = 0;
  }
  *pp = p;
  return rc;
}

static int walIteratorRevInit(Wal *pWal, u32 nBackfill, struct WalIteratorRev *p, u32 mxSafeFrame, int ignoreFrameIfNewerExist){
    WalIterator *pIter;
    u32 *frames;
    u32 iFrame, iPageno;
    int rc;

    frames = (u32*)sqlite3MallocZero((pWal->hdr.mxFrame + 1) * sizeof(u32));
    if (!frames) return SQLITE_NOMEM_BKPT;
    rc = walIteratorInit(pWal, nBackfill, &pIter);
    if (rc || !pIter) {
        sqlite3_free(frames);
        return rc;
    }

    while (walIteratorNext(pIter, &iPageno, &iFrame) == 0) {
        /*
         * If we get a page with a frame_no greater than mxSafeFrame, and ignoreFrameIfNewerExist is false,
         * then we replace it with the latest page with frame_no <= mxSafeFrame.
         */
        if (iFrame > mxSafeFrame && !ignoreFrameIfNewerExist) {
           rc = walFindFrame(pWal, iPageno, mxSafeFrame, &iFrame);
           if( rc!=SQLITE_OK ) break;
           if (iFrame == 0) {
               continue;
           }
        }
        frames[iFrame] = iPageno;
    }
    walIteratorFree(pIter);
    if (rc != 0) {
        sqlite3_free(frames);
        return rc;
    }

    p->current = pWal->hdr.mxFrame;
    p->frames = frames;

    return SQLITE_OK;
}