kriszyp / lmdb-js

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

Crash at LRFUExpirer.delete #114

Closed gogoout closed 2 years ago

gogoout commented 2 years ago

Hi, I noticed crash when I manually set the cacheSize to be something like 500_000. The crash is very reliable to reproduce in the application. The log is like following

TypeError: Cannot set property '1405' of undefined
    at LRFUExpirer.delete (/node_modules/weak-lru-cache/dist/index.cjs:21:67)
    at Map.delete (/node_modules/weak-lru-cache/dist/index.cjs:243:23)
    at Array.<anonymous> (/node_modules/lmdb/caching.js:14:31)
    at afterCommit (/node_modules/lmdb/write.js:448:27)
    at resolveCommit (/node_modules/lmdb/write.js:396:3)
    at resolveWrites (/node_modules/lmdb/write.js:391:5)
    at /node_modules/lmdb/write.js:336:4

However, I wasn't unable to reproduce this crash in a minimal setup (which use the same cacheSize and same lmdb config). I also noticed when I set the cache size lower like the default 30_000, the problem goes away.

My lmdb option is like this:

const db = lmdb.open({
    path: "/tmp/db-test",
    commitDelay: 1,
    useVersions: false,
    encoding: "msgpack",
    sharedStructuresKey: Symbol.for("structures"),
    compression: true,
    cache: {
      cacheSize:500_000
    },
    overlappingSync: true
  });

Sorry can't provide more information. Do you have any idea what may happened? My version is 2.0.0-beta4 I'll see if I can still reproduce this on the latest version

gogoout commented 2 years ago

I tried the latest version (2.1.0-beta2) it still can be reproduce on my application. But the error now can be catched by a try catch. And our automatic retry will still have the save error.

warn:   Error in promise: Cannot set property '26344' of undefined, will retry executing it, left chance [1] times 
warn:   Error in promise: Cannot set property '26351' of undefined, will retry executing it, left chance [0] times 
warn:   Failed in retrying promise for 2 times: Cannot set property '26351' of undefined
kriszyp commented 2 years ago

That's a big cache! The problem is that weak-lru-cache uses 16-bit encoding of the LRU positions (as part of an integer with other flags/bits), so within the four LRUs, that yields a maximum size cache size of 256K. Of course, this limit should actually be checked/reported. And I think I can probably squeeze in a few more bits to permit larger caches.

gogoout commented 2 years ago

Thanks for the clarification, I think checking this and maybe document it is good enough. Thanks.

kriszyp commented 2 years ago

Should be fixed in beta 3.

gogoout commented 2 years ago

No longer reproducible under the beta3, no error message as well.