Kirlovon / aloedb

Light, Embeddable, NoSQL database for Deno 🦕
https://deno.land/x/aloedb
MIT License
140 stars 12 forks source link

await the write #6

Closed ALMaclaine closed 3 years ago

ALMaclaine commented 3 years ago

I'm pretty sure that write should be awaited.

To keep it short, I discovered this in reverse, I had tests that failed because of un-awaited asyncs, so I cloned AloeDB and awaited the write.

I found I had missed some awaits in my tests and code, add them, all tests passed. Next day I figure, we'll the AloeDB comment said its not necessary to await, so I'll switch back to the official package, most tests fail.

Based on this it seems safer to await the write.

Kirlovon commented 3 years ago

Hi! I am not awaiting write method for performance reasons. To avoid creating a complicated data writing method, I just rely on Deno's asynchronous mechanism.

Can you share examples of tests that have been failed?

ALMaclaine commented 3 years ago

The server/aloe db tests in this repo only pass when I await the write.

https://github.com/ALMaclaine/dpm

ALMaclaine commented 3 years ago

Only change it if you think it's an issue, I'm not relying on Aloe for anything serious and will likely swap it out for something else.

Kirlovon commented 3 years ago

The tests fails because test runner checks if all asynchronous processes have finished. This can be easily fixed by changing sanitizeResources and sanitizeOps settings to false:

Deno.test({
  name: "your awesome test",
  sanitizeResources: false,
  sanitizeOps: false,

  async fn() {
    // Your code
  }
});

I think I'll add some kind of parameter for instant data writing, especially for cases like yours.

The database is quite stable, although it is not the final version yet. Right now I'm preparing the final release, most likely it will come out within a month.

ALMaclaine commented 3 years ago

I'm still getting used to Deno, I was aware of being able to change the test settings but wasn't exactly clear what the situation/context was for doing so. Could you enlightenment as to what I should be expecting out of running async tests and under what conditions I may want to set these sanitize settings.

Kirlovon commented 3 years ago

These settings are well described in the official Deno Manual!

As I understand it, the test runner throws an error when the test is completed, but the code continues to perform asynchronous operations.

Since AloeDB is not awaiting writing, your test ends before the AloeDB finishes writing the data. Hope I made it clear 😄

ALMaclaine commented 3 years ago

Thanks, I did read that section of the manual, I just don't understand why I'd want to use those features.

Do you know the consequences or not of this? As in, should I care that my async functions aren't complete when the test over? I know this is outside the scope of the issue, but just looking for some guidance.

ALMaclaine commented 3 years ago

I had a thought, what if you return the promise from the write, then we can choose whether to await it or not, would that work?

Kirlovon commented 3 years ago

There is no need to worry about this, Deno ends its process only when all the asynchronous methods have finished their execution. The only thing you shouldn't do is calling Deno.exit() immediately after inserting, deleting or updating documents, as the writing will take place in the background.

As I said, I'll see what can be done about it. Most likely I will add an await to this method, and add a setting to change the writing behavior.

ALMaclaine commented 3 years ago

Thank you.

Kirlovon commented 3 years ago

As promised, with the last update I added optimize parameter. It's set to true by default, but if you disable it, the database methods will wait until the data writing is done, instead of adding writing to the queue. Here is an example:

import { Database } from 'https://deno.land/x/aloedb/mod.ts';

const db = new Database({
    path: './data.json',
    optimize: false // Optimization disabled
});

// Any CRUD operations
await db.insertOne({ foo: 'bar' });

// Writing is DONE!

const fileData = await Deno.readTextFile('./data.json')
console.log(fileData); // [{ "foo": "bar" }]
ALMaclaine commented 3 years ago

Thank you!