WICG / kv-storage

[On hold] A proposal for an async key/value storage API for the web
Other
551 stars 18 forks source link

Atomic compare-and-swap operation? #56

Open billiegoose opened 5 years ago

billiegoose commented 5 years ago

I recently ran into an issue with race conditions between the main thread and a worker thread clobbering each other's writes using @jakearchibald 's idb-keyval, and decided I needed a function like:

let oldValue: any = await idb.swap(
  key: string,
  expect: (oldValue: any) => boolean,
  value: any
);

as a primitive in order to implement a mutex. Unfortunately, even after forking idb-keyval and hacking on it for a bit, I wasn't able to code such a function correctly, AFAICT. 😭

I know that thread-safety is probably beyond the scope of this module (see #5) but I think it would be nice if a future version could add support for atomic operations.

jakearchibald commented 5 years ago

Off-topic, but I think this would work as an addition to idb-keyval:

export function update(key: IDBValidKey, updater: (val: any) => any, store = getDefaultStore()): Promise<void> {
  return store._withIDBStore('readwrite', store => {
    const req = store.get(key);
    req.onsuccess = () => {
      store.put(updater(req.result), key);
    };
  });
}
billiegoose commented 5 years ago

re: offtopic - Oh that's so much simpler than what I was doing! I was messing around with trying to abort the transaction.

domenic commented 5 years ago

This is very tempting to me, but in the past our storage folks have said that we should not encroach too much on IndexedDB's territory, instead referring people to the backingStore API. I can appreciate the clean separation. WDYT, @asutherland @pwnall and others?

asutherland commented 5 years ago

For the explicit request: I don't think compare-and-swap is appropriate semantics to layer on top of transactions. As I understand the compare-and-swap primitive it is a general low-level multi-threading primitive and in the event of failure values are re-computed and the operation re-attempted. It seems more appropriate to use transactions to address the concurrency issue, as Jake has proposed.

For implementing something like update, I'm open to the idea, but I think it's important to figure out the overall plan as it relates to making IndexedDB more friendly. If we're going to clean up the IndexedDB API to directly support promises and async/await as covered by https://github.com/w3c/IndexedDB/issues/42, that might be the better avenue. For example, the proposed update helper above does not provide the ability to atomically manipulate 2 keys in the same transaction, which leads to foot-guns of its own. My current gut feel is that we should first try and standardize promises and related convenience helpers within the IndexedDB API before growing kv-storage into IDB's territory.

pwnall commented 5 years ago

compare-and-swap seems like a specific case of read-modify-write.

If we do add a primitive, I'd prefer that it's readModifyWrite (or map?). It would start a txn, read a key, call a function with the key's value as an input, then write the function's return value. The function has to be synchronous due to IndexedDB limitations.

I've written readModifyWrite in an IndexedDB wrapper before, so I'm not completely against it. Would like to see a use case though. The mutex problem described in the original post should be solved using the Web Locks API.