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

Host state time was not correctly calculated #2419

Closed
interduo opened this issue Feb 15, 2019 · 26 comments
Closed

Host state time was not correctly calculated #2419

interduo opened this issue Feb 15, 2019 · 26 comments
Labels
bug Undesired behaviour resolved A fixed issue
Milestone

Comments

@interduo
Copy link
Contributor

Describe the bug
There is a broken sorting in host.php form. "In state" column.
in_state_sorting

To Reproduce
Steps to reproduce the behavior:

  1. Go to 'host.php'
  2. Click on 'In state'

Expected behavior
Devices sorted properly by "In state" time.

Screenshots
Did that in first point.

Desktop:

  • OS: [e.g. iOS] - Ubuntu
  • Browser [e.g. chrome, safari] - Chrome
  • Version [e.g. 22] - Newest
@netniV
Copy link
Member

netniV commented Feb 15, 2019

tbh, sorting by in state is almost pointless without sorting by state first. Otherwise, you'll get a mismatch of stats dependant on time. But I can see that you may want to see what device might be constantly changing state.

@interduo
Copy link
Contributor Author

Do You say that this (what I see in screenshot) is expected bahavior?

@netniV
Copy link
Member

netniV commented Feb 15, 2019

Nope, i'm saying that column would not normally be sortable in my mind but I'll see if I can fix it over the weekend.

@interduo
Copy link
Contributor Author

Ok thanks. Now I get Your point.

@netniV
Copy link
Member

netniV commented Feb 19, 2019

OK, so the problem isn't with the sorting but rather the display.

 SELECT host.id,host.hostname, graphs, data_sources, IF(status_event_count>0, status_event_count*300, IF(UNIX_TIMESTAMP(status_rec_date)>943916400,UNIX_TIMESTAMP()-UNIX_TIMESTAMP(status_rec_date), IF(snmp_sysUptimeInstance>0 AND snmp_version > 0, snmp_sysUptimeInstance,UNIX_TIMESTAMP()))) AS instate FROM host LEFT JOIN (SELECT host_id, COUNT(*) AS graphs FROM graph_local GROUP BY host_id) AS gl ON host.id=gl.host_id LEFT JOIN (SELECT host_id, COUNT(*) AS data_sources FROM data_local GROUP BY host_id) AS dl ON host.id=dl.host_id WHERE deleted = '' GROUP BY host.id ORDER BY `instate` ASC LIMIT 0,30;

+----+-------------+--------+--------------+-----------+
| id | hostname    | graphs | data_sources | instate   |
+----+-------------+--------+--------------+-----------+
|  2 | 10.0.0.250  |     17 |           17 |   6114900 |
|  3 | 10.0.0.18   |      8 |           15 |   9475516 |
|  4 | 127.0.0.1   |      1 |            1 | 257353877 |
+----+-------------+--------+--------------+-----------+
3 rows in set (0.00 sec)

image

When I looked at the get_timeinstate() function, I noticed that it was completely ignoring any calculated time and recalculating it itself. So, I wondered what would happen if I corrected this using the following diff:

diff --git a/lib/functions.php b/lib/functions.php
index 8799cc79..62f07146 100644
--- a/lib/functions.php
+++ b/lib/functions.php
@@ -4130,7 +4130,9 @@ function calculate_percentiles($data, $percentile = 95, $whisker = false) {

 function get_timeinstate($host) {
        $interval = read_config_option('poller_interval');
-       if ($host['status_event_count'] > 0) {
+       if (isset($host['instate'])) {
+               $time = $host['instate'];
+       } elseif ($host['status_event_count'] > 0) {
                $time = $host['status_event_count'] * $interval;
        } elseif (strtotime($host['status_rec_date']) > 943916400) {
                $time = time() - strtotime($host['status_rec_date']);

And I ended up with

image

So, the functions are not aligned properly. I will create a patch to rectify that.

netniV added a commit that referenced this issue Feb 19, 2019
Host state time was not correctly calculated if snmp_sysUptimeInstance was used.  The lib/functions.php was dividing the value by 100 but the SQL statement was not, resulting in invalid sort orders.

Additionally, the get_timeinstate() function was updated to utilise the 'instate' element if it exists in the $hosts variable.
@netniV netniV changed the title Broken sorting in host.php, column: "In State" Host state time was not correctly calculated Feb 19, 2019
@netniV
Copy link
Member

netniV commented Feb 19, 2019

This is now in the main development code, please check and close if issue is resolved.

@netniV netniV added bug Undesired behaviour resolved A fixed issue labels Feb 19, 2019
@interduo
Copy link
Contributor Author

I will check that on 29 feb because I am in Jordan.

@cigamit cigamit closed this as completed Feb 24, 2019
@interduo
Copy link
Contributor Author

It's good right now.

@netniV
Copy link
Member

netniV commented Feb 26, 2019

Thanks @interduo

@interduo
Copy link
Contributor Author

interduo commented Mar 1, 2019

I checked it before too less precisely.

It's good right now calculating "in state" field but the sorting is broken as on begining.

in_state

@interduo
Copy link
Contributor Author

interduo commented Mar 1, 2019

We should sort unixtime not the text in field.

@netniV
Copy link
Member

netniV commented Mar 1, 2019

The time is sorted by the value, not the display. Have you all the latest updates?

@interduo
Copy link
Contributor Author

interduo commented Mar 1, 2019

I've got a commit 299f329 in my repo so it should work.

@netniV
Copy link
Member

netniV commented Mar 1, 2019

If lib/function.php's get_daysfromtime() doesn't look something like:

function get_daysfromtime($time, $secs = false, $pad = '', $format = DAYS_FORMAT_SHORT, $all = false) {
        global $days_from_time_settings;

        // Ensure we use an existing format or we'll end up with no text at all
        if (!isset($days_from_time_settings['text'][$format])) {
                $format = DAYS_FORMAT_SHORT;
        }

        $mods = $days_from_time_settings['mods'];
        $text = $days_from_time_settings['text'][$format];

        $result = '';
        foreach ($mods as $index => $mod) {
                if ($mod > 0 || $secs) {
                        if ($time >= $mod) {
                                if ($mod < 1) {
                                        $mod = 1;
                                }
                                $val   = floor($time/$mod);
                                $time %= $mod;
                        } else {
                                $val   = 0;
                        }

                        if ($all || $val > 0) {
                                $result .= padleft($pad, $val, 2) . $text['prefix'] . $text[$index] . $text['suffix'];
                                $all = true;
                        }
                }

It isn't the latest rendition of it.

@interduo
Copy link
Contributor Author

interduo commented Mar 1, 2019

It's the same. Should I clean some cache or rebuild sth?

@netniV
Copy link
Member

netniV commented Mar 1, 2019

Run this:

SELECT 
   host.id,host.hostname, graphs, data_sources, 
   IF(availability_method = 0, 
      'availability',
      IF(status_event_count > 0 AND status IN (1, 2), 
         'status_event_count',
         IF(UNIX_TIMESTAMP(status_rec_date) < 943916400 AND status IN (0, 3), 
            'total_polls',
            IF(UNIX_TIMESTAMP(status_rec_date) > 943916400, 
               'status_rec_date',
               IF(snmp_sysUptimeInstance>0 AND snmp_version > 0,
                  'sysUptimeInstance', 
                  'current_time'
               )
            )
         )
      )
   ) AS method,
   IF(availability_method = 0, 
      '0',
      IF(status_event_count > 0 AND status IN (1, 2), 
         status_event_count*(SELECT IFNULL(value,'300') FROM settings where name = 'poller_interval'),
         IF(UNIX_TIMESTAMP(status_rec_date) < 943916400 AND status IN (0, 3), 
            total_polls*(SELECT IFNULL(value,'300') FROM settings where name = 'poller_interval'),
            IF(UNIX_TIMESTAMP(status_rec_date) > 943916400, 
               UNIX_TIMESTAMP() - UNIX_TIMESTAMP(status_rec_date),
               IF(snmp_sysUptimeInstance>0 AND snmp_version > 0,
                  snmp_sysUptimeInstance/100, 
                  UNIX_TIMESTAMP()
               )
            )
         )
      )
   ) AS instate
FROM host LEFT JOIN (
   SELECT host_id, COUNT(*) AS graphs 
   FROM graph_local 
   GROUP BY host_id
) AS gl 
ON host.id=gl.host_id 
LEFT JOIN (
   SELECT host_id, COUNT(*) AS data_sources 
   FROM data_local 
   GROUP BY host_id
) AS dl 
ON host.id=dl.host_id 
WHERE deleted = '' 
GROUP BY host.id 
ORDER BY `instate` ASC 
LIMIT 0,30;

@netniV
Copy link
Member

netniV commented Mar 1, 2019

Just realised I'd left the $poller_interval in there so it now pulls it from the settings table.

@interduo
Copy link
Contributor Author

interduo commented Mar 1, 2019

+-----+----------------+--------+--------------+-----------------+----------+
| id  | hostname       | graphs | data_sources | method          | instate  |
+-----+----------------+--------+--------------+-----------------+----------+

[cuted]

30 rows in set (0.01 sec)


@netniV
Copy link
Member

netniV commented Mar 1, 2019

SQL is sorting that as a string. Not a number.

@netniV netniV reopened this Mar 1, 2019
@interduo
Copy link
Contributor Author

interduo commented Mar 1, 2019

So the type of 'instate' is mistaken?

@netniV
Copy link
Member

netniV commented Mar 1, 2019

I don't know why, but yes.

@netniV
Copy link
Member

netniV commented Mar 4, 2019

Try this new one:

SELECT 
   host.id,host.hostname, graphs, data_sources, 
   IF(availability_method = 0, 
      'availability',
      IF(status_event_count > 0 AND status IN (1, 2), 
         'status_event_count',
         IF(UNIX_TIMESTAMP(status_rec_date) < 943916400 AND status IN (0, 3), 
            'total_polls',
            IF(UNIX_TIMESTAMP(status_rec_date) > 943916400, 
               'status_rec_date',
               IF(snmp_sysUptimeInstance>0 AND snmp_version > 0,
                  'sysUptimeInstance', 
                  'current_time'
               )
            )
         )
      )
   ) AS method,
   CAST(
      IF(availability_method = 0, 
         '0',
         IF(status_event_count > 0 AND status IN (1, 2), 
            status_event_count*(SELECT IFNULL(value,'300') FROM settings where name = 'poller_interval'),
            IF(UNIX_TIMESTAMP(status_rec_date) < 943916400 AND status IN (0, 3), 
               total_polls*(SELECT IFNULL(value,'300') FROM settings where name = 'poller_interval'),
               IF(UNIX_TIMESTAMP(status_rec_date) > 943916400, 
                  UNIX_TIMESTAMP() - UNIX_TIMESTAMP(status_rec_date),
                  IF(snmp_sysUptimeInstance>0 AND snmp_version > 0,
                     snmp_sysUptimeInstance/100, 
                     UNIX_TIMESTAMP()
                  )
               )
            )
         )
      ) AS unsigned
   ) AS instate
FROM host LEFT JOIN (
   SELECT host_id, COUNT(*) AS graphs 
   FROM graph_local 
   GROUP BY host_id
) AS gl 
ON host.id=gl.host_id 
LEFT JOIN (
   SELECT host_id, COUNT(*) AS data_sources 
   FROM data_local 
   GROUP BY host_id
) AS dl 
ON host.id=dl.host_id 
WHERE deleted = '' 
GROUP BY host.id 
ORDER BY `instate` ASC 
LIMIT 0,30;

This should work for you as it changes my data from

+-----+--------+--------------+-----------------+----------+
| id  | graphs | data_sources | method          | instate  |
+-----+--------+--------------+-----------------+----------+
|  69 |     23 |           22 | status_rec_date | 10539395 |
|  71 |      9 |            8 | status_rec_date | 10556915 |
| 137 |   NULL |         NULL | status_rec_date | 10801835 |
|  73 |     10 |            9 | status_rec_date | 10801835 |
|  89 |      3 |            3 | status_rec_date | 12098975 |
| 104 |      3 |            3 | status_rec_date | 12110255 |
| 100 |      3 |            3 | status_rec_date | 12110615 |
| 127 |      2 |            2 | status_rec_date | 12569795 |
|  79 |     10 |            9 | status_rec_date | 1272875  |
|  70 |     10 |            9 | status_rec_date | 13123355 |
| 115 |      2 |            2 | status_rec_date | 13176875 |
| 117 |      2 |            2 | status_rec_date | 13176875 |
|  75 |      9 |            8 | status_rec_date | 14300675 |
|  31 |      7 |            7 | status_rec_date | 1469735  |
|  44 |     27 |           26 | status_rec_date | 1573835  |
|  90 |      1 |            1 | status_rec_date | 1595435  |
|  39 |      9 |            8 | status_rec_date | 16577375 |
|  83 |      3 |            3 | status_rec_date | 1679135  |
| 116 |      2 |            2 | status_rec_date | 1679735  |
|  82 |      1 |            1 | status_rec_date | 1679735  |
|  80 |      5 |            5 | status_rec_date | 16889675 |
| 133 |      5 |            5 | status_rec_date | 16889675 |
| 132 |     11 |           10 | status_rec_date | 16889675 |
|  78 |      9 |            8 | status_rec_date | 16889675 |
|  98 |      3 |            3 | status_rec_date | 16889675 |
| 131 |     11 |           10 | status_rec_date | 16889675 |
| 109 |      3 |            3 | status_rec_date | 16889675 |
| 111 |      3 |            3 | status_rec_date | 16889915 |
|  42 |     12 |           12 | status_rec_date | 16889915 |
| 135 |      7 |           10 | status_rec_date | 16889975 |
+-----+--------+--------------+-----------------+----------+

to

+-----+--------+--------------+--------------------+---------+
| id  | graphs | data_sources | method             | instate |
+-----+--------+--------------+--------------------+---------+
|  60 |     10 |            9 | status_rec_date    |  249037 |
|  51 |     10 |           10 | status_rec_date    |  249097 |
| 142 |     12 |           12 | status_rec_date    |  297457 |
| 152 |     13 |           13 | status_rec_date    |  297457 |
| 145 |     23 |           23 | status_rec_date    |  297457 |
| 150 |     13 |           13 | status_rec_date    |  297457 |
| 143 |     17 |           17 | status_rec_date    |  297457 |
| 148 |     12 |           12 | status_rec_date    |  297457 |
| 141 |     14 |           13 | status_rec_date    |  297457 |
| 146 |     13 |           13 | status_rec_date    |  297457 |
| 151 |     14 |           14 | status_rec_date    |  297457 |
| 144 |     13 |           13 | status_rec_date    |  297457 |
| 149 |     12 |           12 | status_rec_date    |  297457 |
| 126 |      2 |            2 | status_rec_date    |  297517 |
| 108 |      1 |            1 | status_rec_date    |  297517 |
| 118 |      2 |            2 | status_rec_date    |  297517 |
| 124 |      2 |            2 | status_rec_date    |  297517 |
| 114 |      2 |            2 | status_rec_date    |  297517 |
| 139 |     10 |           10 | status_rec_date    |  298897 |
| 140 |   NULL |         NULL | status_rec_date    |  298957 |
| 103 |      1 |            1 | status_rec_date    |  299017 |
| 112 |      1 |            1 | status_rec_date    |  463357 |
| 147 |   NULL |         NULL | total_polls        |  604800 |
|  74 |     10 |            9 | status_rec_date    |  896017 |
|  87 |      3 |            3 | status_event_count |  935820 |
|  79 |     10 |            9 | status_rec_date    | 1272817 |
|  31 |      7 |            7 | status_rec_date    | 1469677 |
|  44 |     27 |           26 | status_rec_date    | 1573777 |
|  90 |      1 |            1 | status_rec_date    | 1595377 |
|  83 |      3 |            3 | status_rec_date    | 1679077 |
+-----+--------+--------------+--------------------+---------+

@interduo
Copy link
Contributor Author

interduo commented Mar 4, 2019

Now query result is sorted correctly by instate in expected way.
Now the issue trully is resolved ;)

@netniV
Copy link
Member

netniV commented Mar 4, 2019

I have now committed this to the latest development code.

@netniV netniV added this to the v1.2.3 milestone Mar 4, 2019
@interduo
Copy link
Contributor Author

interduo commented Mar 4, 2019

Why did You not close this issue?

@netniV
Copy link
Member

netniV commented Mar 4, 2019

We must have posted around the same time so I missed yours. You can actually close your own issues if you believe them to be resolved 👍

@interduo interduo closed this as completed Mar 5, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Undesired behaviour resolved A fixed issue
Projects
None yet
Development

No branches or pull requests

3 participants