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

Abort DB operations / Transaction using AbortSignal #8552

Open
rluvaton opened this issue Jan 20, 2022 · 8 comments
Open

Abort DB operations / Transaction using AbortSignal #8552

rluvaton opened this issue Jan 20, 2022 · 8 comments

Comments

@rluvaton
Copy link
Contributor

rluvaton commented Jan 20, 2022

Feature Description

The Problem

Aborting DB operations is useful when the user abort some request when the DB operation is no longer needed (e.g. getting result for autocomplete and then the user type more characters (this is not my use case but rather a common use case that need the abort feature))

The Solution

Using AbortSignal to abort DB operation and transaction.

There are 2 approaches I have in mind:

  1. Passing signal
  2. Returning object with abort function

I prefer the 1st option as it keep the library usage design

Usage

When I use signal in the below example I mean an AbortSignal instance

Repository and EntityManager
userRepository.find({
    withDeleted: true,
    signal,
});
QueryBuilder
const firstUser = await connection
    .getRepository(User)
    .createQueryBuilder("user")
    .where("user.id = :id", { id: 1 })
    // here:
    .addSignal(signal)
    .getOne();
Transactions

Using Connection or EntityManager:

// An isolation level can be passed in the object near the signal property
await getManager().transaction({ signal }, async transactionalEntityManager => {

});

Using QueryRunner:

await queryRunner.startTransaction({ signal });

Using decorators:
How this should be used? Maybe can set the abort signal on the passed manager

Passing the signal to the DB transaction will abort the transaction when the signal abort emit, passing

Considered Alternatives

Additional Context

Relevant Database Driver(s)

All of them basically

Are you willing to resolve this issue by submitting a Pull Request?

  • ✖️ Yes, I have the time, and I know how to start.
  • ✖️ Yes, I have the time, but I don't know how to start. I would need guidance.
  • ✖️ No, I don’t have the time, but I can support (using donations) development.
  • ✅ No, I don’t have the time and I’m okay to wait for the community / maintainers to resolve this issue.
@Ginden
Copy link
Collaborator

Ginden commented Feb 2, 2022

I'm not sure if this can be implemented before Node.js and TypeORM drop support for Node 12 (last version without AbortController).

@TheJaredWilcurt
Copy link

Node 12's Maintenance support ends on 2022-04-30 (66 days from now).

Work on this feature could begin now and wait to be merged until after Node 12's EOL, since it is relatively near.

@Ginden
Copy link
Collaborator

Ginden commented Feb 23, 2022

What do you think about placing it in execute method and getOne etc.?

Eg.

class QueryBuilder {
   execute(options?: {signal: AbortSignal});
}
```

@rluvaton
Copy link
Contributor Author

rluvaton commented Feb 23, 2022

What do you think about placing it in execute method and getOne etc.?

Eg.

class QueryBuilder {
   execute(options?: {signal: AbortSignal});
}

Don't you think .addSignal method will be more intuitive given that the whole query builder is like that?

@Ginden
Copy link
Collaborator

Ginden commented Feb 23, 2022

I'm not sure about that. Can you provide links to existing fluent interfaces using AbortSignal?

@thebergamo
Copy link

Every place that I see there isn't any fluent interface around AbortSignal it's mostly simply passing signal as part of the options of a given function that execute operations.

Is still any plan to support this feature?

@andreykrupskii
Copy link

It would be a super helpful feature to optimize performance, especially for systems (like ours) that run heavy analytical queries. Do you know if this feature is in the library roadmap?

@santunioni
Copy link

santunioni commented Apr 26, 2024

Providing signal to execute seems better for three reasons

  • you will abort only something that is running, not the act of building the query. That's my intuition on judging the best interface. This is not an argument for how the code runs, but for how we use it.
  • it seems to be the simplest implementation. The simpler the better
  • it is the simplest interface

Is someone implementing this? I volunteer to take a look and comeback with estimations or a pr if it is fast

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants