Skip to content

Conversation

@odesenfans
Copy link
Collaborator

Improved the incoming function to simplify it and make it more
generic.

Removed the bulk_operation flag as we always wish for bulk DB
operations. The function now returns a status + a list of DB
operations. DB operations are a new class that contains a Mongo
operation and the name of the collection to target.
This avoids passing lists as arguments and modifying them in place.

The loop to await individual message tasks now waits for all tasks
to complete instead of failing on the first operation.
This fixes an issue where, if an exception occurs soon enough in
the batch, the output of still-running tasks could be lost.

Added type hints.

@moshemalawach
Copy link
Member

If all jobs are "big batches", it means they are processed asynchronously... It might get a big delay on p2p-pubsub/ipfs-pubsub incoming messages, leading to non-ui-proof latency.

Current applications are expecting actions to be done sub-500ms after a use clicks on something and signs the message. If we can still process them fast enough it's ok, if not we need to rethink this a bit.

@odesenfans
Copy link
Collaborator Author

@moshemalawach I introduced a bug here indeed, I need to change the logic in the IPFS pubsub part to apply the operations from incoming. Once fixed, this PR will preserve the current behavior. I changed the incoming function to:

  1. make it stateless, which is easier to test
  2. make it more powerful, as we can now specify operations on any collection and not just on pending_messages and messages.

In any case I'm keeping the performance aspect in mind, we already have integration tests on the aleph_client repository that validate the time it takes for a message to propagate on the network.

@odesenfans odesenfans force-pushed the od-refactor-message-processor branch from b70781c to db4ec19 Compare April 7, 2022 08:39
Improved the `incoming` function to simplify it and make it more
generic.

Removed the `bulk_operation` flag as we always wish for bulk DB
operations. The function now returns a status + a list of DB
operations. DB operations are a new class that contains a Mongo
operation and the name of the collection to target.
This avoids passing lists as arguments and modifying them in place.

The loop to await individual message tasks now waits for all tasks
to complete instead of failing on the first operation.
This fixes an issue where, if an exception occurs soon enough in
the batch, the output of still-running tasks could be lost.

Added type hints.
@odesenfans odesenfans force-pushed the od-refactor-message-processor branch from db4ec19 to d65eacf Compare April 7, 2022 08:48
@odesenfans odesenfans added this to the April release - v0.2.2 milestone Apr 7, 2022
@moshemalawach
Copy link
Member

Ok, great to hear :)

@odesenfans odesenfans merged commit 520cd30 into aleph-im:dev Apr 26, 2022
@odesenfans odesenfans deleted the od-refactor-message-processor branch April 26, 2022 09:40
odesenfans added a commit that referenced this pull request Apr 27, 2022
Improved the `incoming` function to simplify it and make it more
generic.

Removed the `bulk_operation` flag as we always wish for bulk DB
operations. The function now returns a status + a list of DB
operations. DB operations are a new class that contains a Mongo
operation and the name of the collection to target.
This avoids passing lists as arguments and modifying them in place.

The loop to await individual message tasks now waits for all tasks
to complete instead of failing on the first operation.
This fixes an issue where, if an exception occurs soon enough in
the batch, the output of still-running tasks could be lost.

Added type hints.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants