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

Accepting disposition hangs if the link is close()d #126

Closed
richardpark-msft opened this issue Feb 10, 2022 · 4 comments · Fixed by #171
Closed

Accepting disposition hangs if the link is close()d #126

richardpark-msft opened this issue Feb 10, 2022 · 4 comments · Fixed by #171
Assignees
Labels
blocking-release Blocks release

Comments

@richardpark-msft
Copy link
Member

richardpark-msft commented Feb 10, 2022

While testing azservicebus it looks like if the link is closed calling 'accept()' will hang, rather than returning a more relevant error (ex: ErrLinkClosed).

This is partly investigation, as @jhendrixMSFT did have code to handle this case (code)

@richardpark-msft richardpark-msft self-assigned this Feb 10, 2022
@jhendrixMSFT jhendrixMSFT added the blocking-release Blocks release label Jun 24, 2022
@richardpark-msft
Copy link
Member Author

I just figured this one out, PR incoming!

richardpark-msft added a commit that referenced this issue Jul 5, 2022
Disposition of a message would hang if the link was closed prior to the settlement function being called.

The problem is that we don't do a link check to see if our link is valid anymore, which can lead to:

1. Receive message
2. Link is detached
3. Settle message

For message settlement all the traffic happens through the session and connection, not necessarily the link, which means that if the link is detached or dead we have a disposition that has a channel that we never close, resulting in a hang.

Fixes #126
@mcardy
Copy link

mcardy commented Oct 14, 2022

I'm hitting this issue on 0.17.5. Can we tag 0.17.6 with this fix @jhendrixMSFT?

@mgevantmakher
Copy link

mgevantmakher commented Oct 14, 2022

@richardpark-msft @jhendrixMSFT
Hello Richard hello Joel I'm also hitting it, pls tag a release with this fix

mcardy added a commit to SolaceDev/opentelemetry-collector-contrib that referenced this issue Oct 14, 2022
Working around Azure/go-amqp#126 by batching dispositions

SOL-79806
@richardpark-msft
Copy link
Member Author

@mgevantmakher, this is the internally vendored copy of go-amqp. Are you looking for this fix to appear in the Service Bus package (ie: azservicebus) or in go-amqp itself?

If it's within azservicebus, the latest version of azservicebus already contains this fix.

djaglowski pushed a commit to open-telemetry/opentelemetry-collector-contrib that referenced this issue Oct 18, 2022
…ith Solace PubSub+ Event Broker 10.2.0 (#15244)

* Use modify message instead of reject message to indicate failure

SOL-75279

* Set delivery mode on propagated spans

SOL-75196

* Added character type to user property mapping

SOL-75145

* Changed dropped_user_properties to dropped_application_message_properties

SOL-74920

* Updated protobuf spec, incorporated transactional changes

SOL-75362

* Scoped metrics to receiver instances based on instance name if present

SOL-75870, SOL-74853

* Made delivery mode mapped strings lowercase

SOL-75196

* Use string of length 1 instead of int for character value mapping

SOL-75145

* Gracefully handle enums beyond the current known set

This allows forward compatibility in the event that new entries are added in the future

SOL-78861

* Use the slice type in the attribute map when mapping user properties

OpenTelemetry+backends do not support the Bytes type, and instead support a slice of integers

SOL-78624

* Revert "Use the slice type in the attribute map when mapping user properties"

This reverts commit 48b9730.

* Change name of Enqueue Event in OpenTelemetry Spans to always be '<endpoint name> enqueue', even for anonymous endpoints

Also removes messaging.destination as an attribute on enqueue events as its now redundant

SOL-79098

* Handle the case of an empty payload, returning an error

In this case, the message will be dropped and stats incremented

SOL-79796

* Fixed an issue causing AcceptMessage to hang indefinitely

Working around Azure/go-amqp#126 by batching dispositions

SOL-79806

* Revert "Fixed an issue causing AcceptMessage to hang indefinitely"

This reverts commit bf61326.
shalper2 pushed a commit to shalper2/opentelemetry-collector-contrib that referenced this issue Dec 6, 2022
…ith Solace PubSub+ Event Broker 10.2.0 (open-telemetry#15244)

* Use modify message instead of reject message to indicate failure

SOL-75279

* Set delivery mode on propagated spans

SOL-75196

* Added character type to user property mapping

SOL-75145

* Changed dropped_user_properties to dropped_application_message_properties

SOL-74920

* Updated protobuf spec, incorporated transactional changes

SOL-75362

* Scoped metrics to receiver instances based on instance name if present

SOL-75870, SOL-74853

* Made delivery mode mapped strings lowercase

SOL-75196

* Use string of length 1 instead of int for character value mapping

SOL-75145

* Gracefully handle enums beyond the current known set

This allows forward compatibility in the event that new entries are added in the future

SOL-78861

* Use the slice type in the attribute map when mapping user properties

OpenTelemetry+backends do not support the Bytes type, and instead support a slice of integers

SOL-78624

* Revert "Use the slice type in the attribute map when mapping user properties"

This reverts commit 48b9730.

* Change name of Enqueue Event in OpenTelemetry Spans to always be '<endpoint name> enqueue', even for anonymous endpoints

Also removes messaging.destination as an attribute on enqueue events as its now redundant

SOL-79098

* Handle the case of an empty payload, returning an error

In this case, the message will be dropped and stats incremented

SOL-79796

* Fixed an issue causing AcceptMessage to hang indefinitely

Working around Azure/go-amqp#126 by batching dispositions

SOL-79806

* Revert "Fixed an issue causing AcceptMessage to hang indefinitely"

This reverts commit bf61326.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocking-release Blocks release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants