Unqlite lost some key-value pair after commit

[onto]

I found strange bug, for some keys unqlite lost some key-value pair after commit, I write code for reproduce it:

#include <string>
#include <ctime>
#include <cstdint>

extern "C" {
#include "../UnQLite-kv/unqlite.h"
}

struct TestStruct
{
    double d1;
    double d2;
};

int main(int argc, char **argv)
{
    std::string filename("TEST." + std::to_string(time(nullptr)));
    TestStruct test = { 1.0, 1.1 };

    // Fill database 
    {
        unqlite *db = nullptr;

        int res = unqlite_open(&db, filename.c_str(), UNQLITE_OPEN_CREATE);
        if (res != UNQLITE_OK)
        {
            return res;
        }

        for (int64_t i = 0; i < 165; ++i)
        {
            int res = unqlite_kv_store(db, &i, sizeof(int64_t), &test, sizeof(TestStruct));
            if (res != UNQLITE_OK)
            {
                return res;
            }
        }

        res = unqlite_close(db);
        if (res != UNQLITE_OK)
        {
            return res;
        }
    }

    // Reopen
    {
        unqlite *db = nullptr;

        int res = unqlite_open(&db, filename.c_str(), UNQLITE_OPEN_CREATE);
        if (res != UNQLITE_OK)
        {
            return res;
        }

        // Write pair with key 162 again
        int64_t i_bug = 162;
        res = unqlite_kv_store(db, &i_bug, sizeof(int64_t), &test, sizeof(TestStruct));
        if (res != UNQLITE_OK)
        {
            return res;
        }

        // Force commit
        res = unqlite_commit(db);
        if (res != UNQLITE_OK)
        {
            return res;
        }

        // Test pair with key 164, it lost
        int64_t i = 164;
        TestStruct test1= {};
        unqlite_int64 buf_size = sizeof(TestStruct);
        res = unqlite_kv_fetch(db, &i, sizeof(int64_t), &test1, &buf_size);
        if (res != UNQLITE_OK)
        {
            printf("%d", res); //Yep, print -6 there
            return res;
        }

        res = unqlite_close(db);
        if (res != UNQLITE_OK)
        {
            return res;
        }
    }

    return 0;
}

For this types of key and value, after reopen database and write pair with key 162 and force commit unqlite lost pair with key 164.
I check it with last unqlite, unqlite-kv and vedis sources on msvc2013, clang 3.8.1 and gcc 6.2.0

[buaabyl]

When you store kv reverse order.

        for (int64_t i = N - 1; i >= 0; --i)
        {
            int res = unqlite_kv_store(db, &i, sizeof(int64_t), &test, sizeof(TestStruct));
            if (res != UNQLITE_OK)
            {
                return res;
            }
        }

then key "0" is missing instead of key "164", is there any more tests?

[buaabyl]

I digging some codes, maybe there are some dirty page not update?

fist store key and values, there are page with number: 0, 1, 2, 3, 4, 5, 6, 7.
then close and open, there are page: 0, 1, 2, 4, 7
when update key 162, it found cell in page 7, and data in page 4,
and it trigger page overflow, then it create a new page 5. so page 4 and 5 is dirty.

pager_get_dirty_pages call page_merge_dirty process dirty page,
and the return list is page 4, page 5. wrote that list to disk.
and page 4, 5 is unref.

but page 7 untouch......

when you unref page 7, it will reload page 7, and every thing will be fine.

the fastest fix is release all pages.
or we have to find whether a bug exist of update dirty page.
maybe is

static int lhRecordOverwrite(
	lhcell *pCell,
	const void *pData,unqlite_int64 nByte
	)

modify:

static int pager_commit_phase1(Pager *pPager)

	/* Write the dirty pages */
	rc = pager_write_dirty_pages(pPager,pDirty);
	if( rc != UNQLITE_OK ){
		/* Rollback your DB */
		pPager->iFlags |= PAGER_CTRL_COMMIT_ERR;
		pPager->pFirstDirty = pDirty;
		unqliteGenError(pPager->pDb,"IO error while writing dirty pages, rollback your database");
		return rc;
	}

    /* release all pages */
    {
        Page *p;

        while (1) {
            p = pPager->pAll;
            if (p == NULL) {
                break;
            }
            pager_unlink_page(pPager, p);
        }
    }
    /* release all pages */

	/* If the file on disk is not the same size as the database image,
     * then use unqliteOsTruncate to grow or shrink the file here.
     */
	if( pPager->dbSize != pPager->dbOrigSize ){
		unqliteOsTruncate(pPager->pfd,pPager->iPageSize * pPager->dbSize);
	}
	/* Sync the database file */
	unqliteOsSync(pPager->pfd,UNQLITE_SYNC_FULL);
	/* Remove stale flags */
	pPager->iJournalOfft = 0;
	pPager->nRec = 0;
	return UNQLITE_OK;

[buaabyl]

or is there mismatch of zData and pUserData?

typedef struct Page Page;
struct Page {
  unsigned char *zData;           /* Content of this page */
  void *pUserData;                /* Extra content */
......

[symisc]

I think releasing all dirty pages is the quick fix for this issue. But I've to check on that once back to the office.
Meanwhile, could you submit a pull request for further investigation?
Thanks

[symisc]

This obscure bug is now identified in the hash disk key/value store. The defragment function only uses the cell array on the current page, rather than the master page.

4809c66#diff-d7673e675c496e14016c07fb72df2a57

[onto]

This bug is still reproducible after 4809c66 fix, please check and reopen issue

The lhPageDefragment function not called whatever in this test code

[symisc]

@onto @buaabyl

The bug in the pager layer is fixed as of 1.1.8. You can test and report back. I've tested against your code and everything should be fine with the 1.1.8 fix.

[onto]

Yes, this bug is gone now, thanks