Skip to content

Conversation

kenliao94
Copy link
Contributor

Add design doc for implementing Jakarta Messaging 3.1 asynchronous send per the latest process discussed on dev list.

Note to reviewers, GitHub rich diff doesn't render image properly. You can click "view file" from the hamburger button to see the rendered markdown.

@jbonofre jbonofre self-requested a review February 19, 2025 07:25
@cshannon
Copy link
Contributor

I don't really like the use of images as they are not easily modifiable. Markdown supports code blocks (and you can mark it as java) so I would use something like that instead.

Copy link
Member

@jbonofre jbonofre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image is fine for design diagram/schema.
However, it should not be used for code.
Instead we should use the markdown code block: the huge advantage is that we can suggest change during the review.

@@ -0,0 +1,225 @@
# Jakarta Messaging 3.1: asynchronous send design doc

Date: Jan 5th 2025
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not add date here (the PR date is good enough 😄 ).

@cshannon
Copy link
Contributor

Image is fine for design diagram/schema. However, it should not be used for code. Instead we should use the markdown code block: the huge advantage is that we can suggest change during the review.

+1 for this

@kenliao94
Copy link
Contributor Author

Thanks @cshannon and @jbonofre for the feedback, I will use code block instead. Hoping to publish a new revision tmr.

@kenliao94
Copy link
Contributor Author

Use codeblock instead of image.

@kenliao94 kenliao94 requested a review from jbonofre February 25, 2025 23:18
@cshannon
Copy link
Contributor

I haven't had a chance to get back to looking at this yet but still plan to, @mattrpav have you had a change to review this yet?


**Approach 2: Application thread submits the async send to a work queue, a single background thread will dequeue a message from the queue, send it asynchronously and wait for it to finish, before dequeuing the next.**

This is similar to approach 1, the async message is sent one at a time. The difference being instead of doing it in the application thread, the `Session` object will have a `SingleThreadExecutor` to handle sending of the messages on a background thread. The advantage of doing that is the application thread is not blocked by sending an async message, it simply enqueues the message to the work queue (ExecutorService) then proceeds. Allowing it to batch send a lot of async messages at a time without blocking.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

each send on the executor would still need a round trip to the server on this case.
you are only unblocking the sender's thread., right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wouldn't be possible to have a completion callback on the storage, sending responses to the client upon callback?

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.

4 participants