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

Loading speed improvement when viewing syslogs for specific device #7062

Merged
merged 4 commits into from Jul 25, 2017

Conversation

Projects
None yet
4 participants
@EnzoZafra
Contributor

EnzoZafra commented Jul 20, 2017

When trying to view the syslogs for a specific device
(for example http://[hostname]/device/device=[device_id]/tab=logs/section=syslog/)
the dropdown for 'priorities' and 'programs' are not filtered to search only for that specific device_id. This causes the SQL query to be extremely slow and sometimes causes the browser to crash when there are lots of syslog entries in the database.

Also, since the syslog entries in the table themselves are filtered by device_id, I think that the 'programs' and 'priorities' dropdown should be as well; to reflect the entries in the syslog table.

DO NOT DELETE THIS TEXT

Please note

Please read this information carefully. You can run ./scripts/pre-commit.php to check your code before submitting.

Testers

If you would like to test this pull request then please run: ./scripts/github-apply <pr_id>, i.e ./scripts/github-apply 5926

@florianbeer

This comment has been minimized.

Show comment
Hide comment
@florianbeer

florianbeer Jul 21, 2017

Contributor

Nice! I am going to try this out right now.

Contributor

florianbeer commented Jul 21, 2017

Nice! I am going to try this out right now.

@florianbeer

This comment has been minimized.

Show comment
Hide comment
@florianbeer

florianbeer Jul 21, 2017

Contributor

Hmm, can't really say it's faster, but it is certainly much cleaner.
Especially having all of the programs in the dropdown, even the ones that are not relevant to this device, always bothered me a lot. Now it is all neat and pristine 👍

Definitely a vote from me to merge this in!

Contributor

florianbeer commented Jul 21, 2017

Hmm, can't really say it's faster, but it is certainly much cleaner.
Especially having all of the programs in the dropdown, even the ones that are not relevant to this device, always bothered me a lot. Now it is all neat and pristine 👍

Definitely a vote from me to merge this in!

Show outdated Hide outdated html/pages/syslog.inc.php
@EnzoZafra

This comment has been minimized.

Show comment
Hide comment
@EnzoZafra

EnzoZafra Jul 24, 2017

Contributor

@florianbeer Yeah, it only helps with speed when there are thousands of programs that aren't related to the device. We found that even though there are only say 5 syslog entries for the device it loads as slow as the overall syslog page.

Contributor

EnzoZafra commented Jul 24, 2017

@florianbeer Yeah, it only helps with speed when there are thousands of programs that aren't related to the device. We found that even though there are only say 5 syslog entries for the device it loads as slow as the overall syslog page.

@florianbeer

This comment has been minimized.

Show comment
Hide comment
@florianbeer

florianbeer Jul 24, 2017

Contributor

Yeah, it only helps with speed when there are thousands of programs that aren't related to the device. We found that even though there are only say 5 syslog entries for the device it loads as slow as the overall syslog page

Yea, that makes sense. Gets me thinking though: why do you have thousand of different programs? Maybe we can improve the detection and organisation of those strings?

Contributor

florianbeer commented Jul 24, 2017

Yeah, it only helps with speed when there are thousands of programs that aren't related to the device. We found that even though there are only say 5 syslog entries for the device it loads as slow as the overall syslog page

Yea, that makes sense. Gets me thinking though: why do you have thousand of different programs? Maybe we can improve the detection and organisation of those strings?

@EnzoZafra

This comment has been minimized.

Show comment
Hide comment
@EnzoZafra

EnzoZafra Jul 24, 2017

Contributor

Look at this table: https://pastebin.com/WApYR69S
It looks like all of the devices with > 1000 distinct programs has an 'ios' OS, so it probably has something to do with how its parsed.

When I searched through the syslog for device_id = 1224 (device with 165623 distinct programs) this is a snippet of what I found: https://pastebin.com/p0p2jwrE
Now if we fix the issue in our switches that creates these syslog entries, those programs wouldn't be created in the first place.
The first part of the line is the 'program'. I'm not sure if this is intended but it looks like its creating an index for each entry and therefore creating thousands of distinct programs..

Contributor

EnzoZafra commented Jul 24, 2017

Look at this table: https://pastebin.com/WApYR69S
It looks like all of the devices with > 1000 distinct programs has an 'ios' OS, so it probably has something to do with how its parsed.

When I searched through the syslog for device_id = 1224 (device with 165623 distinct programs) this is a snippet of what I found: https://pastebin.com/p0p2jwrE
Now if we fix the issue in our switches that creates these syslog entries, those programs wouldn't be created in the first place.
The first part of the line is the 'program'. I'm not sure if this is intended but it looks like its creating an index for each entry and therefore creating thousands of distinct programs..

@florianbeer

This comment has been minimized.

Show comment
Hide comment
@florianbeer

florianbeer Jul 24, 2017

Contributor

That is definitely not intended. I suggest you open a new issue, so we can get to the bottom of this. Shouldn't be to hard to fix the parser.

Contributor

florianbeer commented Jul 24, 2017

That is definitely not intended. I suggest you open a new issue, so we can get to the bottom of this. Shouldn't be to hard to fix the parser.

Show outdated Hide outdated html/pages/syslog.inc.php

EnzoZafra and others added some commits Jul 24, 2017

@scrutinizer-notifier

This comment has been minimized.

Show comment
Hide comment
@scrutinizer-notifier

scrutinizer-notifier Jul 25, 2017

The inspection completed: No new issues

scrutinizer-notifier commented Jul 25, 2017

The inspection completed: No new issues

@laf

laf approved these changes Jul 25, 2017

@laf laf merged commit faaa94a into librenms:master Jul 25, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details

@EnzoZafra EnzoZafra deleted the EnzoZafra:syslog-queryfix branch Jul 28, 2017

@lock

This comment has been minimized.

Show comment
Hide comment
@lock

lock bot May 17, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed.

lock bot commented May 17, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed.

@lock lock bot locked as resolved and limited conversation to collaborators May 17, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.