Skip to content
This repository has been archived by the owner on Oct 12, 2023. It is now read-only.

Message's DeliveryCount should not be modified when a message is peeked #589

Closed
gmantri opened this issue Oct 18, 2018 · 6 comments
Closed
Labels
Milestone

Comments

@gmantri
Copy link

gmantri commented Oct 18, 2018

Actual Behavior

  1. Message delivery count is increased by 1

Expected Behavior

  1. Message delivery count should not increase

This is more of a clarification question. Let's say I create a queue and send 1 message to that queue. Immediately after sending the message, I fetch that message using PeekAsync().

It is my understanding that because I am just peeking the message and not locking it, the delivery count of that message should not change and I am expecting the delivery count to be 0. However what I am noticing is that the delivery count of that message is 1. If we look at the code here, you will notice that SDK is adding 1 to the value returned by the Service Bus.

I am curious to know why SDK is doing this.

@SeanFeldman
Copy link
Collaborator

SeanFeldman commented Oct 18, 2018

Given that AmqpMessageConverter is a generic helper class, it doesn't differentiate between use cases (receive vs peek). Therefore this "issue" manifests itself. Saying that, delivery count is meaningless when message is peeked 🙂

@gmantri
Copy link
Author

gmantri commented Oct 18, 2018

Thanks Sean. I am curious to know why you say that delivery count is meaningless in this case. Wouldn't it create confusion in borderline cases? For example, if I have max delivery count is set as 10 and the actual delivery count is 9 but SDK is reporting it as 10 (because it adds 1 to it).

@SeanFeldman
Copy link
Collaborator

What I meant by meaningless is that in the context of a peek operation, there's no delivery. Therefore count doesn't matter. From a correctness point of view, that's a silly bug and should be addressed. Let me fix it.

@gmantri
Copy link
Author

gmantri commented Oct 18, 2018

Thanks!

@gmantri gmantri closed this as completed Oct 18, 2018
@SeanFeldman SeanFeldman reopened this Oct 18, 2018
@SeanFeldman
Copy link
Collaborator

Re-opened so that when the fix is merged this would get closed.

@SeanFeldman SeanFeldman changed the title Confusion about message's delivery count value when peeking messages Message's DeliveryCount should not be modified when a message is peeked Oct 18, 2018
@SeanFeldman
Copy link
Collaborator

PR is pending.

@nemakam nemakam added this to the 3.2.0 milestone Nov 20, 2018
nemakam pushed a commit that referenced this issue Nov 20, 2018
Fixes #589

* Verification test
* Differentiate between peek and receive operations
* Indicate peek operation
* Additional test case to verify behavior when message is received
* Test delivery count on peek against real queue
* Test delivery count on peeklock against real queue
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants