Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[service-bus] Change MessageSession over to using the .NET receiveMessages algorithm #10107

Conversation

richardpark-msft
Copy link
Member

[sessions] Changing our receiveMessages() algorithm to match .NET's.

The new version works like this:

receive(maxMessages, maxWaitTime)

Internally we then wait for:

  1. maxMessages to arrive
  2. maxWaitTime to expire
    or
  3. An internal 1 second timeout that will fire after the first message
    has been received or the remaining time left from maxWaitTime (whichever one
    is smaller)

Complements #9968 and is the final change for #9718

…r (should just be an equivalent call, just tucked into a function)
- Made what _should_ be a harmless change - moved addCreditAndSetTimer _above_ the registration of the event handlers, making it more like the batchingreceiver. It also just gives me a simple signal to say "hey, everything is registered now".

Also able to do some nice code deletions:
- Removed the old "message wait" timer that wasn't getting used anywhere but receiveMessages
- Removed sessionCallee - we don't have a session manager anymore anyways so it's useless (ie, it's always "standalone")
- newMessageWaitTimeoutInMs is only used locally in the promise. Demoted to be a local variable instead.
- Like in the non-session MessageReceiver I killed the `resetTimerOnNewMessageReceived` functions for streaming - the timeout value was never set so it was essentially a no-op.
… BatchingReceiver tests except `setupFakeReceiver` doesn't add the .session EventEmitter to the mock receiver - not used by MessageSession.
…-session cases in tests with just a little bit more code to simulate drain.
@richardpark-msft
Copy link
Member Author

/azp run js - servicebus - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@richardpark-msft
Copy link
Member Author

/azp run js - servicebus - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@richardpark-msft
Copy link
Member Author

/azp run js - servicebus - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@richardpark-msft
Copy link
Member Author

/azp run js - servicebus - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@richardpark-msft richardpark-msft merged commit 5d544e8 into Azure:master Jul 17, 2020
@richardpark-msft richardpark-msft deleted the richardpark-sb-track2-receive-alg-sessions branch July 17, 2020 23:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants