Skip to content

Span the hardware inventory collection over a period of time.#199

Merged
r0h4n merged 2 commits intoTendrl:masterfrom
nnDarshan:patch-5
Feb 2, 2017
Merged

Span the hardware inventory collection over a period of time.#199
r0h4n merged 2 commits intoTendrl:masterfrom
nnDarshan:patch-5

Conversation

@nnDarshan
Copy link
Copy Markdown
Contributor

@nnDarshan nnDarshan commented Feb 2, 2017

Currently the node inventory is synced every 3 seconds which is very aggressive, considering they change very rarely. So Its better to change the sync interval. Also instead of syncing everything after certain interval (say 5 or 10 mins) of time which causes a huge spike in CPU utilization every time this interval is reached, We can span the inventory collection over a period of time.

In this patch I have introduced a gevent sleep of 10 secs after collecting every resource. So Its like every resource is updated every 3 minutes once.
tendrl-bug-id: #198

@nnDarshan
Copy link
Copy Markdown
Contributor Author

@brainfunked @r0h4n @nthomas-redhat @shtripat Please review

Copy link
Copy Markdown
Member

@shtripat shtripat left a comment

Choose a reason for hiding this comment

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

LGTM.
Comments from others required.

Copy link
Copy Markdown
Contributor

@r0h4n r0h4n left a comment

Choose a reason for hiding this comment

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

Looks good, but how does it affect performance and alerting cc @anmolbabu (please review)

@anmolbabu
Copy link
Copy Markdown
Contributor

anmolbabu commented Feb 2, 2017

@r0h4n None of these impact performance monitoring because that is in total done for us by collectd. But, in future when we add service status monitoring, we might only see a lag in the time of it being noticed and that very very minute and I think it is very much tolerable. And given that it is not delaying the first time update(Its only that interval b/w subsequent syncs has changed) specially of node context, the change looks good to me.

@r0h4n
Copy link
Copy Markdown
Contributor

r0h4n commented Feb 2, 2017

@nnDarshan What is the cpu consumption with this PR?

r0h4n
r0h4n previously approved these changes Feb 2, 2017
@r0h4n r0h4n dismissed their stale review February 2, 2017 09:48

on request by PR owner

Comment thread tendrl/node_agent/node_sync/__init__.py Outdated
try:
gevent.sleep(3)
interval = 10
if tendrl_ns.first_node_inventory_sync == True:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

a simple "if tendrl_ns.first_node_inventory_sync" is enough

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

Comment thread tendrl/node_agent/node_sync/__init__.py Outdated
if s.running:
tags.append(TENDRL_SERVICE_TAGS[service.strip("@*")])
s.save()
gevent.sleep(interval)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

do not sleep for every iteration of this loop, put this after the loop

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

Currently the node inventory is synced every 3 seconds which
is very aggressive, considering they change very rarely.
So Its better to change the sync interval. Also instead
of syncing everything after certain interval
(say 5 or 10 mins) of time which causes a huge spike in CPU
utilization every time this interval is reached,
We can span the inventory collection over a period of time.

In this patch I have introduced a gevent sleep of 10 secs after
collecting every resource. So Its like every resource is
updated every 3 minutes once.

tendrl-bug-id: Tendrl#198
Signed-off-by: nnDarshan <darshan.n.2024@gmail.com>
@nnDarshan
Copy link
Copy Markdown
Contributor Author

@r0h4n Earlier in the machine I tested CPU util was continuously 40-50%... now with this fix and Tendrl/commons#151 its always below 1% and when we fetch inventory it spikes a little to around 5-10%

@r0h4n r0h4n merged commit cc67051 into Tendrl:master Feb 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants