Updating js-db to 5.x.x (integrating RocksDB and DB-native concurrency control)

[emmacasolin]

Description

Updating CI/CD including:

• Continuous benchmarking
• Staging branch
• Advanced type checking

Related:

• Integrate js-db and concurrency testing Polykey#419
• Fine Grained Concurrency Control Standardisation Polykey#294
• Use fast-check to test concurrency consistency invariants Polykey#382

Tasks

• 1. Create new staging branch
• 2. Synchronise changes from js-logger
• 3. update @matrixai/db to 5.0.1,
• 4. fix API changes for DB
• 3. Fix failing tests
• 4. Integrate fast-check and update any relevant tests.

Final checklist

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

[emmacasolin]

The problem with updating js-logger here (which we need to do in order to update it in PK) is that it also means we need to update the DB, but this also means incorporating all of the new (breaking) changes from 5.0.0 of js-db. I think I've made the correct changes to the source code, but there are several failing tests. This is to be expected though, since js-db has undergone some major changes. Most of the failures are to do with concurrency, which makes sense given the focus of most of the recent changes to the db.

[CMCDragonkai]

Hold on this PR, the js-db update is more pressing first. And we don't need to update the EFS yet. The js-db 5.0.0 is first being updated in PK. EFS can stay on the original js-db.

[CMCDragonkai]

What you can do is to merge in the non-js-db and non-js-logger changes in first. There's just a bunch of CI/CD stuff.

Can you split up the commit into a separate PR for just the staging and ci/cd and other chores.

[emmacasolin]

Hold on this PR, the js-db update is more pressing first. And we don't need to update the EFS yet. The js-db 5.0.0 is first being updated in PK. EFS can stay on the original js-db.

Ok. EFS won't work inside PK if it's running the old DB though, since the old DB uses the old logger, which conflicts with the new logger used by the new DB in PK, so it will need to be updated here eventually.

[CMCDragonkai]

Actually you can do it. You just won't be able to pass a "child" logger into the DB that EFS is using. So you can just remove that parameter entirely.

[emmacasolin]

Now that I've updated the logger for all of the other libraries though we can no longer pass a logger through to any of the other libraries from EFS. I think this is only an issue for js-workers though, which is used only in the benchmarks and in one of the tests (efsWorker.test.ts), so we could just not pass a logger into those.

[emmacasolin]

ee0b9e2 now contains all of the CI/CD changes but doesn't update the logger or DB. As such, it doesn't pass a logger through to any of the workers since js-workers uses a different version of js-logger. All of the tests should be passing on that commit, but they don't pass on the most recent commit due to concurrency changes from the latest DB.

[CMCDragonkai]

Once #68 is merged, rename this PR to focus on js-db 5.x upgrade. Logger changes should be eliminated here. Since that will be put into #68. Although once rebased, you can also remove the comment.

[CMCDragonkai]

@emmacasolin please update to using 1.3.6 of js-workers. It is now compatible with swc.

The quickfix is to swap from using new X(); to new this(); wherever we have static async creators.

This needs to be fixed in EncryptedFS.ts. Change to:

    const efs = new this({

To make it usable. Then swc should be enabled.

[CMCDragonkai]

The same fix has to be applied to PK as well under staging branch. Can you do it at the same time as you do you js-logger updates there.

[tegefaulkes]

0084570 fixes #69. There still are failing tests in this PR but it passed when tested on top of staging.

[CMCDragonkai]

Creation of temporary directory with os.tmpdir() should still be using path.join as normal.

[CMCDragonkai]

@emmacasolin this PR needs to be split up. The js-db changes should be moved into a new PR.

This PR should only focus on synchronising changes, updating the js-logger, and the fix that @tegefaulkes just added.

[CMCDragonkai]

I suspect that the test failures are coming from the js-db changes. The js-db update should be postponed till later. @tegefaulkes

[CMCDragonkai]

Actually I was confused, the recent fix should go to #68 and #67. Will rename this PR to suit.

@tegefaulkes can you reset the commit you just pushed?

[CMCDragonkai]

When this is being worked on all:

      // @ts-ignore - version of js-logger is incompatible (remove when js-db updates to 5.* here)

Should be removed from the source, tests and benchmarks as js-logger will be compatible and we'd have completed the upgrade.

Unlike in Polykey where it has a commit that we can revert, this PR won't have that convenience.

[tegefaulkes]

Removed the fix commit from here and pushed it to staging.

[tegefaulkes]

Fast-check can be used to combine a number of concurrency tests together. For example, concurrent file writes using a number of methods can be the same test. for these tests the the result matters less than avoiding transaction conflicts. we still want to check that something has been written however.

[tegefaulkes]

The INodeManager is using a LockBox along side the the database transactions. I think we will want to make use of the transaction locks now.

FileDescriptor has its own Lock for locking on cursor updates. This one should be left alone.

[tegefaulkes]

This should be good to merge now. Pending review if desired.

[CMCDragonkai]

Can you rerun the benchmarks and show the comparison here. Since the db has changed, I'm expecting it should be alot faster.

[CMCDragonkai]

Oh yea you need to provide the type of the lock. So don't worry.

[CMCDragonkai]

Ok so for all the concurrent tests where you have replaced the test, change to using a regular expression to match the original predicates on the output. I've given some examples above.

For the new tests, this can also be done if you see an easy way to write the regular expression. Prefer this over simply checks like 3.

Then proceed to merge to staging.

On the staging branch, regenerate docs and redo the benchmarks.

[CMCDragonkai]

There's a documented way of doing delaying the actual async functions for the fast check scheduler: dubzzz/fast-check#564. See the relevant commits. I also asked a question upstream to see how it should be done.

The docs also got moved around recently and the relevant commits point to: https://github.com/dubzzz/fast-check/blob/main/packages/fast-check/documentation/RaceConditions.md

There's no inbuilt helper for this.

The docs suggest:

const scheduleCall = <T>(s: Scheduler, f: () => Promise<T>) => {
  s.schedule(Promise.resolve("USE THIS AS A LABEL...")).then(() => f());
}
// Calling doStuff will be part of the task scheduled in s
scheduleCall(s, () => doStuff())

It's very similar to what you already did with the gateFactory.

Using the above method you also don't need a separate factory.

The last interesting thing is fc.scheduledModelRun. But there is no documentation how to use it. It seems to provide a way of model based testing which requires a sort of state machine model first then you're essentially doing transitions in lock step between the abstract and the real machine. This can be used for EFS, but it would only be done at a very high level considering the entire FS state as a model. This would be a completely different set of tests. Best used for more "consistent" systems (like distributed databases). Not really the EFS in this case. We could apply this to certain stateful domains inside PK though. For example the OCL concepts and ACL.

[tegefaulkes]

RIght, just need to squash and checks. Then this will be ready to merge.

[CMCDragonkai]

Did you check the rest of the tests you replaced to ensure that no predicates are missing? I didn't go through all of them.

[tegefaulkes]

Yes, I went through all of the ones I updated.

[tegefaulkes]

Cleaned up the commit history.

[tegefaulkes]

Just waiting on CI now.

[CMCDragonkai]

I've asked about the integrating timeouts between fast check and jest: dubzzz/fast-check#3084