Skip to content
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

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

Merged
merged 5 commits into from
Jun 25, 2023

Conversation

CMCDragonkai
Copy link
Member

@CMCDragonkai CMCDragonkai commented Jun 23, 2023

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

  • 1. Investigate the test failures in DBTransaction with the new 4.0.0 of @matrixai/async-locks
  • 2. Update to using Monitor inside the DBTransaction
  • 3. Optional enabling of the deadlock detection system
  • 4. Update all other project structure and test for Node 18.15.

Final checklist

  • Domain specific tests
  • Full tests
  • Updated inline-comment documentation
  • Lint fixed
  • Squash and rebased
  • Sanity check the final build

@CMCDragonkai
Copy link
Member Author

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
Copy link
Member Author

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
Copy link

ghost commented Jun 23, 2023

👇 Click on the image for a new way to code review

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map legend

@CMCDragonkai
Copy link
Member Author

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 CMCDragonkai force-pushed the feature-new-locks-monitor branch from daf7f4a to bea669d Compare June 25, 2023 05:41
@CMCDragonkai CMCDragonkai force-pushed the feature-new-locks-monitor branch from bea669d to 7717f9f Compare June 25, 2023 05:42
@CMCDragonkai
Copy link
Member Author

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 CMCDragonkai force-pushed the feature-new-locks-monitor branch from 15d1cf9 to 950bfad Compare June 25, 2023 09:07
@CMCDragonkai CMCDragonkai changed the title WIP: Updating js-db project structure and integrating 4.0.0 of @matrixai/async-locks Updating js-db project structure and integrating 4.0.0 of @matrixai/async-locks Jun 25, 2023
@CMCDragonkai
Copy link
Member Author

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

@CMCDragonkai CMCDragonkai merged commit 7ba545b into staging Jun 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Deadlock Detection
1 participant