kriszyp / lmdb-js

Simple, efficient, ultra-fast, scalable data store wrapper for LMDB
Other
505 stars 42 forks source link

Error in store.ifNoExists throws ReferenceError: ifVersion is not defined #23

Closed kylebernhardy closed 3 years ago

kylebernhardy commented 3 years ago

My script creates an environment and 2 dbis and I am testing out the store.ifNoExists function in a loop:

const fs = require('fs');
const path = require('path');
const lmdb = require('lmdb-store');
const casual = require('casual');

function createEnv(name){
    let environment_path = path.join(__dirname, 'data', name);
    try {
        fs.rmdirSync(environment_path, {recursive:true})
        fs.mkdirSync(environment_path);
    }catch(e){
        console.error(e);
    }
    let env_init = {
        "path": environment_path,
        "mapSize": 1048576,
        "maxReaders": 100
    };
    let env = lmdb.open(env_init);
    return env;
}

let env = createEnv('test');

let id_dbi = env.openDB('id', {
    create: true,
    dupSort: false,
    useVersions: true});

let name_dbi = env.openDB('name', {
    create: true,
    dupSort: true,
    useVersions: false});

let promises = [];

for(let x = 1, length = 10000; x<length; x++){
    let name = casual.first_name;
    let prom = id_dbi.ifNoExists(x, ()=>{
        id_dbi.put(x, {id: x, name: name});
        name_dbi.put(name, x);
    });
}

Promise.allSettled(promises).then(d=>{console.log(d);
    for (let { key, value } of id_dbi.getRange({ start:3, end: 6 })) {
        console.log(key, value);
    }
});

It throws the error:

/usr/bin/node /home/kyle/WebstormProjects/lmdb-testing/index.js /home/kyle/WebstormProjects/lmdb-testing/node_modules/lmdb-store/index.js:270 return ifVersion(key, null, callback) ^

ReferenceError: ifVersion is not defined at LMDBStore.ifNoExists (/home/kyle/WebstormProjects/lmdb-testing/node_modules/lmdb-store/index.js:270:4) at Object. (/home/kyle/WebstormProjects/lmdb-testing/index.js:40:23) at Module._compile (internal/modules/cjs/loader.js:999:30) at Object.Module._extensions..js (internal/modules/cjs/loader.js:1027:10) at Module.load (internal/modules/cjs/loader.js:863:32) at Function.Module._load (internal/modules/cjs/loader.js:708:14) at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:60:12) at internal/main/run_main_module.js:17:47

kriszyp commented 3 years ago

Ok, simple fix, sorry about the bad reference, fixed in 0.9.7. BTW, quick note about your code: it doesn't look like you are accumulating any promises in the promises array. However, since all the puts to a db are sequential (or simultaneous), you could just await on the last promise returned before proceeding to getRange (rather than needing to await them all with all or allSettled).

kylebernhardy commented 3 years ago

Thanks Kris!

Good to know about the order of operations for the puts and evaluating just the last promise. This was me doing a janky load test, good news, for a single process, I am getting ~100k of those puts in about 900ms!!! This is fantastic.

kriszyp commented 3 years ago

Glad to hear you are getting good results. A few things about these results:

Because all of the serialization/encoding work occurs outside the transaction, the (single mutex protected) write transaction length is shorter which means this should scale well across multiple threads/processes. When I have run my benchmarks across 8 threads, I can get over 3x as many total writes per second, whereas if you ran this same benchmark with all writes occurring within a transaction on the main thread, you probably will get almost no speedup with more threads.

I'd also expect you could probably further improve performance a bit (and space efficiency) by using a shared structure (enabled with sharedStructureKey).

And this performance is also using the default sync setting, which means safety/durability. I would argue that noSync is just not suitable for production use. We have experience data corruption using it, and I have been able to readily reproduce data corruption by powering off a VM under load with that setting.

kylebernhardy commented 3 years ago

Thanks Kris,

The performance with sync enabled is what I am very excited about. I will add the sharedStructuresKey to my environment creation as well.