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

Properly close internal reply queues #9488

Merged
merged 2 commits into from
Jun 9, 2021
Merged

Properly close internal reply queues #9488

merged 2 commits into from
Jun 9, 2021

Conversation

coignetp
Copy link
Contributor

@coignetp coignetp commented Jun 8, 2021

What does this PR do?

Close internal reply queue when collecting queue stats.

We tried to fix this in #8349 , but customer didn't test the beta release. Here is the original explaination of the fix:

A customer reported that the Agent writes to a dead-letter queue (DLQ) associated to PYMQPCF.*. PYMQPCF.* is the prefix of internal queues (known as "reply queues") used by pymqi when executing a command.

  • Under normal conditions, the command succeeds and pymqi pops messages from those queues to form the final response, leaving no undelivered messages and nothing in the DLQ.
  • But if an exception occurs, some messages may be left in the reply queue, and because we don't dispose it explicitly they will be left hanging there, marked as "undelivered", and moved to a DLQ. By calling pcf.disconnect(), the reply queue gets properly closed, so no messages will be marked as "undelivered" since the queue is disposed.

Motivation

Support case

Additional Notes

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • PR title must be written as a CHANGELOG entry (see why)
  • Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
  • PR must have changelog/ and integration/ labels attached

@coignetp coignetp requested a review from a team as a code owner June 8, 2021 10:51
@ghost ghost added the integration/ibm_mq label Jun 8, 2021
@coignetp coignetp marked this pull request as draft June 8, 2021 11:10
@coignetp coignetp marked this pull request as ready for review June 8, 2021 13:32
@codecov
Copy link

codecov bot commented Jun 8, 2021

Codecov Report

Merging #9488 (c05da10) into master (aae3d8e) will not change coverage.
The diff coverage is n/a.

Impacted Files Coverage Δ
...checks/ibm_mq/collectors/queue_metric_collector.py 90.90% <ø> (ø)
ibm_mq/tests/common.py 100.00% <0.00%> (ø)
ibm_mq/tests/test_utils.py 100.00% <0.00%> (ø)
ibm_mq/tests/test_metadata.py 100.00% <0.00%> (ø)
ibm_mq/tests/test_ibm_mq_int.py 100.00% <0.00%> (ø)
ibm_mq/tests/test_ibm_mq_unit.py 100.00% <0.00%> (ø)
ibm_mq/datadog_checks/ibm_mq/utils.py 100.00% <0.00%> (ø)
ibm_mq/datadog_checks/ibm_mq/errors.py 100.00% <0.00%> (ø)
ibm_mq/datadog_checks/ibm_mq/__init__.py 100.00% <0.00%> (ø)
ibm_mq/datadog_checks/ibm_mq/__about__.py 100.00% <0.00%> (ø)
... and 15 more

# https://github.com/dsuch/pymqi/blob/084ab0b2638f9d27303a2844badc76635c4ad6de/code/pymqi/__init__.py#L2892-L2902
# https://dsuch.github.io/pymqi/examples.html#how-to-specify-dynamic-reply-to-queues
if pcf is not None:
pcf.disconnect()
Copy link
Contributor

Choose a reason for hiding this comment

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

any chances this can throw an exception?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@coignetp coignetp merged commit 9d395c0 into master Jun 9, 2021
@coignetp coignetp deleted the paul/ibm-mq-pcf branch June 9, 2021 16:05
mx-psi pushed a commit that referenced this pull request Jul 27, 2021
I did not cherry-pick release PRs (#9708, #9492, #9456)
nor documentation-only PRs (#9418, #9616, #9642) from git
history to keep the commit as small as possible.

PR #9400 is also relevant but it was already on the git history
of this branch (see `git log -- ibm_mq` to double check).

Don't emit any warnings if NO_MSG_AVAILABLE is received (#9452)

(cherry picked from commit 13c10d9)

Properly close internal reply queues (#9488)

* Properly close internal reply queues

* Define pcf

(cherry picked from commit 9d395c0)

Add debug line when there are no messages available (#9702)

* Add debug line when there are no messages available

(cherry picked from commit 3055228)

Do not submit critical service check when there are no messages (#9703)

(cherry picked from commit f0568c1)
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