Add action processing queues #146
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ai
reviewed
Aug 29, 2023
ai
reviewed
Aug 30, 2023
ai
reviewed
Aug 30, 2023
ai
reviewed
Aug 30, 2023
ai
reviewed
Aug 30, 2023
base-server/index.js
Outdated
| @@ -355,6 +369,45 @@ export class BaseServer { | |||
| this.listenNotes = {} | |||
| bindBackendProxy(this) | |||
|
|
|||
| let end = (meta, queue, queueKey, ...args) => { | |||
| this.actionToQueue.delete(meta.id) | |||
| if (queue?.length() === 0) { | |||
There was a problem hiding this comment.
Good question, it should never be missed at this point, but it was.
The issue was that inside on('processed'), I determine whether an action belongs to the queue by using actionToQueue map.
I save an action to this map by meta.id, but inside on('processed') I lookup the map by meta.id and action.id (because I may catch logux/undo, which has different meta.id but might have action.id of some action in the queue, in which case I need to undo all other actions in that queue).
However, after processing, I always deleted an action from the actionToQueue by meta.id, so if it was logux/undo action, the following happened:
on('processed')gotlogux/undowithaction.id = '1'andmeta.id = '2'actionToQueuehas an action withmeta.id = '1'equal toaction.id = '1'so we processlogux/undo- we undo all remaining actions in this queue and clear the queue
- we try to delete this action (
logux/undo) from theactionToQueuemap bymeta.id = '2'and delete nothing - the queue is deleted because it is empty (all actions are undone and removed from it)
on('processed')got some other action withmeta.id = '1'actionToQueuehas an action withmeta.id = '1'so we process it- we try to use the queue which was deleted in step
5
Instead, in step 4 we should have deleted an action from actionToQueue using action.id so that in step 7 an action would be ignored because it was undone.
Co-authored-by: Andrey Sitnik <[email protected]>
Co-authored-by: Andrey Sitnik <[email protected]>
Co-authored-by: Andrey Sitnik <[email protected]>
ai
reviewed
Aug 31, 2023
ai
reviewed
Sep 1, 2023
base-server/index.js
Outdated
| end(meta.id, queue, queueKey, e) | ||
| }) | ||
| this.on('processed', (action, meta) => { | ||
| let queueKey = this.actionToQueue.get(action?.id) |
There was a problem hiding this comment.
You can’t check action.id until you check action.type. Diferent actions could have a very dfferent data in action with possible collisions.
So add logux/processed or something here. But I think we should check meta.id to mark tasks as processed on their own processed event, not on processed event of logux/processed action for that action.
ai
added a commit
that referenced
this pull request
Oct 8, 2023
* add queues * fix some tests * comment out broken tests * remove unused commented code * remove TODO * comment out one more test because it is failing * fix one test * fix one more test * fix one more test * fix one more test * remove commented code * fix bind-backend-proxy test by adding setQueue method * fix one more test * use setQueue inside base-server * add type for onActions callback * add type and docs for setQueue * add types and docs for #type and #channel * fix pnpm-lock * remove eslint-disable-no-console * change test delay * change test delay * change test delay * fix test * always unbind error if processed * fix tests, add test * add test * make sure that all actions are processed before destroy * import actionsCallback type from core * add test that checks channel regex * add test that checks channel pattern * add test to check that error in one queue does not affect another queue * add test: "undoes all other actions in a queue if some action should be undone" * add test that checks setQueue method * remove empty queues from the map * replace ignoreDestroying with actionsInQueue set * update onActions * check for duplicate meta.id * rename onActions to onSync * refactor (rename, simplify code), use VladBrok/core * optimize by binding single event listeners for all queues instead of for each action * adjust logic when queue is undefined * add test to check that an action with same ID is not added to the queue * Update base-server/index.js Co-authored-by: Andrey Sitnik <[email protected]> * Update base-server/index.js Co-authored-by: Andrey Sitnik <[email protected]> * Update base-server/index.js Co-authored-by: Andrey Sitnik <[email protected]> * code style fix * fix lint error * fix missing queue * use updated onSync api that allows to call access in a queue * update VladBrok/core * fix test * fix test * use only action.id in on(processed) for queues, fix destroying logic * remove setQueue, change tests accordingly * check action.type before checking action.id * rename onSync to onReceive --------- Co-authored-by: Andrey Sitnik <[email protected]>
ai
added a commit
that referenced
this pull request
Feb 10, 2024
* add queues * fix some tests * comment out broken tests * remove unused commented code * remove TODO * comment out one more test because it is failing * fix one test * fix one more test * fix one more test * fix one more test * remove commented code * fix bind-backend-proxy test by adding setQueue method * fix one more test * use setQueue inside base-server * add type for onActions callback * add type and docs for setQueue * add types and docs for #type and #channel * fix pnpm-lock * remove eslint-disable-no-console * change test delay * change test delay * change test delay * fix test * always unbind error if processed * fix tests, add test * add test * make sure that all actions are processed before destroy * import actionsCallback type from core * add test that checks channel regex * add test that checks channel pattern * add test to check that error in one queue does not affect another queue * add test: "undoes all other actions in a queue if some action should be undone" * add test that checks setQueue method * remove empty queues from the map * replace ignoreDestroying with actionsInQueue set * update onActions * check for duplicate meta.id * rename onActions to onSync * refactor (rename, simplify code), use VladBrok/core * optimize by binding single event listeners for all queues instead of for each action * adjust logic when queue is undefined * add test to check that an action with same ID is not added to the queue * Update base-server/index.js Co-authored-by: Andrey Sitnik <[email protected]> * Update base-server/index.js Co-authored-by: Andrey Sitnik <[email protected]> * Update base-server/index.js Co-authored-by: Andrey Sitnik <[email protected]> * code style fix * fix lint error * fix missing queue * use updated onSync api that allows to call access in a queue * update VladBrok/core * fix test * fix test * use only action.id in on(processed) for queues, fix destroying logic * remove setQueue, change tests accordingly * check action.type before checking action.id * rename onSync to onReceive --------- Co-authored-by: Andrey Sitnik <[email protected]>
ai
added a commit
that referenced
this pull request
Feb 15, 2024
* add queues * fix some tests * comment out broken tests * remove unused commented code * remove TODO * comment out one more test because it is failing * fix one test * fix one more test * fix one more test * fix one more test * remove commented code * fix bind-backend-proxy test by adding setQueue method * fix one more test * use setQueue inside base-server * add type for onActions callback * add type and docs for setQueue * add types and docs for #type and #channel * fix pnpm-lock * remove eslint-disable-no-console * change test delay * change test delay * change test delay * fix test * always unbind error if processed * fix tests, add test * add test * make sure that all actions are processed before destroy * import actionsCallback type from core * add test that checks channel regex * add test that checks channel pattern * add test to check that error in one queue does not affect another queue * add test: "undoes all other actions in a queue if some action should be undone" * add test that checks setQueue method * remove empty queues from the map * replace ignoreDestroying with actionsInQueue set * update onActions * check for duplicate meta.id * rename onActions to onSync * refactor (rename, simplify code), use VladBrok/core * optimize by binding single event listeners for all queues instead of for each action * adjust logic when queue is undefined * add test to check that an action with same ID is not added to the queue * Update base-server/index.js Co-authored-by: Andrey Sitnik <[email protected]> * Update base-server/index.js Co-authored-by: Andrey Sitnik <[email protected]> * Update base-server/index.js Co-authored-by: Andrey Sitnik <[email protected]> * code style fix * fix lint error * fix missing queue * use updated onSync api that allows to call access in a queue * update VladBrok/core * fix test * fix test * use only action.id in on(processed) for queues, fix destroying logic * remove setQueue, change tests accordingly * check action.type before checking action.id * rename onSync to onReceive --------- Co-authored-by: Andrey Sitnik <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes logux/logux#54
Depends on logux/core#52 (will not work properly until logux/core#52 is merged)
This PR adds action processing queues.
A developer can specify the name of the queue for different actions and channels using Server#type and Server#channel.
If the name is not specified, the default 'main' will be used.
If a developer wants to run actions in parallel, they can add them to different queues.
Queues are created for each client, for example, if a client with id '10' creates the 'main' queue and a client with id '11' creates the 'main' queue, these would be different queues even though they have the same name, because the client ids are different.
Queues for channels and actions are always different, even if they have the same name.
if an error occurs during action processing, the queue will undo all it's other actions.