Skip to content

Optionally disable ACK and REP responses to SET requests#21

Open
klanclos wants to merge 20 commits intomainfrom
no_ack
Open

Optionally disable ACK and REP responses to SET requests#21
klanclos wants to merge 20 commits intomainfrom
no_ack

Conversation

@klanclos
Copy link
Contributor

@klanclos klanclos commented Feb 4, 2026

This pull request disables all responses from the server that would normally be triggered in response to a SET request. If a normal blocking SET request can be issued 5000 times per second, a client not waiting for a full response can instead issue 5500 requests per second, a client not waiting for any response can issue 40000 requests per second.

@klanclos klanclos marked this pull request as draft February 4, 2026 00:16
@klanclos klanclos marked this pull request as ready for review February 4, 2026 00:32
@klanclos klanclos marked this pull request as draft February 4, 2026 19:06
@klanclos klanclos marked this pull request as ready for review February 4, 2026 19:43
@klanclos klanclos changed the title Optionally disable ACK responses to SET requests Optionally disable ACK and REP responses to SET requests Feb 4, 2026
Copy link
Contributor

@tylertucker202 tylertucker202 left a comment

Choose a reason for hiding this comment

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

I'm seeing a lot of repeated code. Would it be possible to implement a function here?

def get_reply(message):
    try:
        reply = message.payload.reply
    except SomeExpectedException:
        reply = True
    return reply

Then you can reuse this method wherever.

contents but I missed a few of the names.
@klanclos
Copy link
Contributor Author

klanclos commented Feb 7, 2026

I'm leaning towards setting up the logic as a property in the Payload class, which would eliminate the need to carry the what's a reasonable-default logic around.

@klanclos klanclos marked this pull request as draft February 7, 2026 01:30
@klanclos
Copy link
Contributor Author

That looks cleaner to me. I still need to test it more thoroughly, then I'll move it out of draft status.

@klanclos klanclos marked this pull request as ready for review February 10, 2026 19:39
@klanclos
Copy link
Contributor Author

Tests check out, there doesn't appear to be a performance impact from handling the 'reply' attribute as a property.

Copy link

@mkb0517 mkb0517 left a comment

Choose a reason for hiding this comment

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

These changes look good.

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