MatrixAI / js-db

Key-Value DB for TypeScript and JavaScript Applications
https://polykey.com
Apache License 2.0
5 stars 0 forks source link

Updating js-db project structure and integrating 4.0.0 of `@matrixai/async-locks` #62

Closed CMCDragonkai closed 1 year ago

CMCDragonkai commented 1 year ago

Description

The project structure of PK projects have been changing bringing in things like swc for testing, updates to the eslint structure, updates to library versions and switching to node 18.15. The biggest change will be the @matrixai/async-locks due to its replacement of async-mutex with its own semaphore and introducing Monitor. The Monitor system can replace some of what we are already doing inside the DBTransaction and in fact was based on it.

Issues Fixed

Tasks

Final checklist

CMCDragonkai commented 1 year ago

Currently using 4.0.0 results in some DBTransaction tests to fail:

    ✕ read and write locking (20056 ms)
    ✓ locks are unlocked in reverse order (76 ms)
    ✓ lock re-entrancy (56 ms)
    ✕ locks are isolated per transaction (20078 ms)
    ✕ deadlock (20071 ms)

It mostly applies to the lock system, so that makes sense. Most likely due to changes to LockBox...

We may also proceed to fix this by replacing the LockBox usage and directly using the Monitor instead and integrating the deadlock detection into it as well as option.

CMCDragonkai commented 1 year ago

This will be important before we propagate the 4.0.0 async-locks to everywhere that uses it. I've already done it pre-emptively for async-init though @tegefaulkes.

ghost commented 1 year ago
👇 Click on the image for a new way to code review #### [![Review these changes using an interactive CodeSee Map](https://s3.us-east-2.amazonaws.com/maps.codesee.io/images/github/MatrixAI/js-db/62/6696481b/ad61b14c62ed36dd6d4bf23d7d91187f38e37dff.svg)](https://app.codesee.io/r/reviews?pr=62&src=https%3A%2F%2Fgithub.com%2FMatrixAI%2Fjs-db) #### Legend CodeSee Map legend
CMCDragonkai commented 1 year ago

Ok turns out the problem was just that we no longer just take 0 as the timeout. We have to use the ContextTimerInput with { timer: 0 } literal supported.

Which kind of reminds me, I believe js-contexts still needs to have that brought in even if we haven't yet integrated the Monitor.

CMCDragonkai commented 1 year ago

Cool, so those problems are fixed for now. But I need to also update to just using the Monitor directly without the DBTransaction doing its own thing here. Would align the behaviour I think.

CMCDragonkai commented 1 year ago

There is now a deadlock property that can be enabled when you create the DB.createDB({ deadlock: true }) that will switch on automatic deadlock detection. But switch it off once you're done cause it slows down things.

Furthermore, the API is the same, but any timeouts must be done with a context.

So:

await tran.lock('foo');
await tran.lock(['foo', 'write']);
await tran.lock(['foo', 'write', { timer: 0 }]);
await tran.lock('foo', 'bar');
await tran.lock('foo', ['bar', { timer: 0 }]);
await tran.lock('foo', { timer: 0 });

All work.

Note that the last one, applies the ctx to all locking keys. They end up all sharing the same ctx. Whereas the other cases are passing a single ctx into particular locking key, but not the other locking keys.

@tegefaulkes @amydevs