-
Notifications
You must be signed in to change notification settings - Fork 30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
rw lock for batches #116
Comments
Currently I am seeing some stuff where I have two batches running concurrently, they both do a read and then a write. I see some atomicity issues when running that. Is that the issue you're describing here? Thanks. |
@makew0rld Can you try provide a minimal test case? Otherwise let me know what is the exact atomicity issue so I can try |
I'm struggling to reproduce the issue with a test case. I'll report back if I ever get it working. The issue I encountered had to do with reading an array or object, and then adding to it and writing it back. Two functions would do that at the same time, and I wanted to rely on batches to apply each of their reads and writes sequentially, one function after another. In my experience this didn't happen, but I can't reproduce it with a test case right now. |
@makew0rld I would suggest reading how current batch/lock works, sadly it didn't got merged because API might change so don't use the .lock method or don't necessarily depend on this but read more here: Edit: This was the PR #126 |
I like your use-case, would be awesome if we can make it work, can you share any code that you currently have? |
Here's some test code I wrote, and it is working fine. It's possible my issue is elsewhere. But I'm curious if @mafintosh can comment on what this issue is about? Currently what exactly is "not safe"? Even if it's not related to the issue I'm experiencing, knowing what's unsafe will help guide how I write my hyperbee programs in the future. import Hypercore from "hypercore";
import Hyperbee from "hyperbee";
const core = new Hypercore("test.hypercore");
await core.ready();
const db = new Hyperbee(core, {
keyEncoding: "utf-8",
valueEncoding: "utf-8",
});
async function thread(name, delay) {
const batch = db.batch();
await batch.lock();
const result = await batch.get("test");
if (result === null) {
await new Promise((resolve) => setTimeout(resolve, delay));
await batch.put("test", JSON.stringify([0]));
await batch.flush();
return;
}
const arr = JSON.parse(result.value);
arr.push(arr.at(-1) + 1);
await new Promise((resolve) => setTimeout(resolve, delay));
await batch.put("test", JSON.stringify(arr));
await batch.flush();
}
await Promise.all([thread("thread1", 10), thread("thread2", 10)]);
console.log((await db.get("test")).value); The actual code that caused the issue is open source, but not very accessible to try out: https://github.com/starlinglab/authenticated-attributes/blob/102b629f6ac99b3d680f3b14226145e7ce0d3e9b/hyperbee/import.js#L174-L192 |
You're using Does this test represents your case? #131 |
@makew0rld For example, this is currently unsafe: #110 |
I believe I was using it before, but possibly not. As I said the test case works, so maybe my issue in production was unrelated to this. What makes #110 unsafe? It looks like everything is synchronous because |
Unsafe in that specific case means it throws an error but it shouldn't i.e. it's a bug, we will fix soon The test does 5000 iterations, but if you do let's say 1000 then bug doesn't happen, thus it's generally unsafe because it works until it doesn't haha |
The question is more like what things to consider when using the batch API, besides the previous bug, the only undocumented thing is: And you already know how to use |
Ok, thanks. I am manually using locking, so it should be fine. I'll report back here if I am able to reproduce any issues. |
Oh also it seems undocumented that batches that also do |
currently concurrent read and write ops on a single batch is not safe, it should be
The text was updated successfully, but these errors were encountered: