Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

Fixes TypeError: Cannot read property 'pop' of undefined #530

Merged
merged 1 commit into from
Jan 14, 2020

Conversation

davidmurdoch
Copy link
Member

This fixes an issue caused by writing AND reading the same key multiple times simultaneously. Sometimes the read operation would end up reading the value as 0 bytes due to the way writes are performed in node. To fix this, we implemented a queue so only a single read or write operation can occur at a time for each key; basically an in-memory lock.

Additionally, during testing we found that it was possible for a write operation to fail due to program termination. This failure would sometimes cause the key to exist but contain 0 bytes (which is always invalid). To fix this we write to a temp file, and only if it works do we move this temp file to its correct key location. This prevents early termination from writing partial/empty values.

Of course, all this will eventually be for nothing as we are migrating the db to actual an leveldb implementation that doesn't use a separate file for every key Soon(TM).

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 82.208% when pulling 8e5fe67 on fix/pop into a53c02d on develop.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 82.208% when pulling 8e5fe67 on fix/pop into a53c02d on develop.

Copy link
Contributor

@nicholasjpaterno nicholasjpaterno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@@ -3,6 +3,8 @@ var AbstractLevelDOWN = require("abstract-leveldown").AbstractLevelDOWN;
var async = require("async");
var fs = require("fs");
var path = require("path");
var tmp = require("tmp");
tmp.setGracefulCleanup();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Comment on lines +20 to +39
const accessQueue = {
next: (lKey) => {
const cont = accessQueue.cache[lKey].shift();
if (cont) {
cont();
} else {
delete accessQueue.cache[lKey];
}
},
execute: (lKey, callback) => {
let cache = accessQueue.cache[lKey];
if (cache) {
cache.push(callback);
} else {
accessQueue.cache[lKey] = [];
callback();
}
},
cache: {}
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 on the abstraction. Much cleaner

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants