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

Add selected_message_queues flag to filter message queues and improve performances #12808

Merged
merged 20 commits into from
Sep 28, 2022

Conversation

amenasria
Copy link
Member

@amenasria amenasria commented Aug 23, 2022

What does this PR do?

This PR adds a selected_message_queues flag to allow watching only a specific list of message queues and improve performances.

Motivation

(Customer request)
Sometimes you just want to monitor messages on specific IBM i system message queues. The thing is that the way it's implemented now, the request is being made on all MESSAGE_QUEUE_NAME instead of first filtering by interesting MESSAGE_QUEUE_NAME and after that get the message queue info we want. We suspect the absence of filter to be responsible for the high CPU usage on IBM i hosts so we need to fix that.

Benchmark

To check that the feature really improved the performance I ran some performance tests:

We will be monitoring the QSYSOPR message queue and CECUSER (the user) message queue (those names are not important, we just want to have 2 different message queues). We will create messages in the CECUSER message queue and measure the query execution time on both QSYSOPR, CECUSER, without filter. We will be satisfied if the time for QSYSOPR remains nearly constant, QSYSOPR increases and the without filter increases too

The query is

SELECT MESSAGE_QUEUE_NAME, MESSAGE_QUEUE_LIBRARY, COUNT(*), SUM(CASE WHEN SEVERITY >= 50 THEN 1 ELSE 0 END) FROM QSYS2.MESSAGE_QUEUE_INFO {message_queues_filter} GROUP BY MESSAGE_QUEUE_NAME, MESSAGE_QUEUE_LIBRARY

And the message_queues_filter will take the value:

  • WHERE MESSAGE_QUEUE_NAME IN ('QSYSOPR') for the QSYSOPR query.
  • WHERE MESSAGE_QUEUE_NAME IN ('CECUSER') for the CECUSER query.
  • for the unfiltered query.

Now that we're all on the same page, here are the time results about the query:

(Tests ran on IBM i 7.4 PowerVM POWER9 LPAR 1 vCPU 2048 Mo RAM)

# CECUSER Jobs QSYSOPR query time CECUSER query time No filter query time
0 0.67s 0.36s 0.45s
220 0.40s 0.53s 0.48s
960 0.60s 0.62s 0.62s
2150 0.43s 0.72s 0.72s
3600 0.38s 0.97s 0.91s
6700 0.43s 1.24s 1.20s
9300 0.48s 1.70s 1.57s
10830 0.35s 1.66s 1.64s
12000 0.46s 1.86s 1.85s
66400 0.86s 8.46s 8.64s

So our expectations finally realise as we see the QSYSOPR query time being constant compared to the other two queries. This means less load on the CPU for the filtered query, mission completed !

Additional Notes - Scripts for reproducibility

IBM i is not a Unix-like OS, so I think it's important to detail the scripts used, both for rigor and for reproducibility purposes.

To create the jobs on the VM the following command was used:

max=1000
index=0
while [ $index -lt $max ] ; do
 let index+=1
 echo Job $index
 system "SBMJOB JOBD(QBATCH) JOB(WSYS) JOBQ(QBATCH) CMD(WRKSYSSTS)"
done

To measure the time taken during a query the following command was used:

qsh -c "db2 -t  \"select distinct current_timestamp from sysibm.sysdummy1;\";" | head -n 4 |tail -n 1; qsh -c "db2 \"$query\""; qsh -c "db2 -t  \"select distinct current_timestamp from sysibm.sysdummy1;\";" | head -n 4 |tail -n 1

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

@github-actions
Copy link

The validations job has failed; please review the Files changed tab for possible suggestions to resolve.

@github-actions
Copy link

github-actions bot commented Sep 2, 2022

The validations job has failed; please review the Files changed tab for possible suggestions to resolve.

@github-actions
Copy link

github-actions bot commented Sep 5, 2022

The validations job has failed; please review the Files changed tab for possible suggestions to resolve.

@codecov
Copy link

codecov bot commented Sep 5, 2022

Codecov Report

Merging #12808 (f8ddbea) into master (eecf6d8) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Flag Coverage Δ
ibm_i 82.28% <100.00%> (+0.70%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@amenasria amenasria marked this pull request as ready for review September 9, 2022 13:09
@amenasria amenasria requested review from a team as code owners September 9, 2022 13:09
@@ -287,5 +291,7 @@ def query_map(config: InstanceConfig):
"job_memory_usage": get_job_memory_usage(config.job_query_timeout),
"memory_info": get_memory_info(config.query_timeout),
"job_queue": get_job_queue_info(config.query_timeout),
"message_queue_info": get_message_queue_info(config.system_mq_query_timeout, config.severity_threshold),
"message_queue_info": get_message_queue_info(
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add some tests for this?

Copy link
Member Author

Choose a reason for hiding this comment

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

You're right ! I just finished adding them ! PTAL

Copy link
Member

@mx-psi mx-psi left a comment

Choose a reason for hiding this comment

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

Implementation looks great, just one question about the configuration structure

ibm_i/assets/configuration/spec.yaml Outdated Show resolved Hide resolved
mx-psi
mx-psi previously approved these changes Sep 14, 2022
@github-actions
Copy link

The validations job has failed; please review the Files changed tab for possible suggestions to resolve.

yzhan289
yzhan289 previously approved these changes Sep 26, 2022
Copy link
Contributor

@yzhan289 yzhan289 left a comment

Choose a reason for hiding this comment

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

LGTM. Just 1 non-blocking comment!

ibm_i/tests/test_sql_queries.py Outdated Show resolved Hide resolved
@amenasria amenasria merged commit 3102b92 into master Sep 28, 2022
@amenasria amenasria deleted the amenasria/ibm-i-select-msg-queues branch September 28, 2022 20:39
steveny91 pushed a commit that referenced this pull request Oct 27, 2022
…ve performances (#12808)

Add selected_message_queues flag + tests
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