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

Hosts are being incorrectly filtered when no location filter is set #3424

Closed
gdsotirov opened this issue Apr 7, 2020 · 24 comments
Closed

Hosts are being incorrectly filtered when no location filter is set #3424

gdsotirov opened this issue Apr 7, 2020 · 24 comments
Assignees
Labels
bug Undesired behaviour confirmed Bug is confirm by dev team gui UI related issue resolved A fixed issue
Milestone

Comments

@gdsotirov
Copy link
Contributor

Describe the bug

After upgrade to 1.2.11 most of the devices are not listed in Console -> Management -> Devices without any filters used. All filters are cleared, but I notice that Location acts awkward. I'm not able to select anything and the select box is blank.

To Reproduce

Steps to reproduce the behavior:

  1. Upgrade to 1.2.11 (in my case from 1.2.10).

  2. Go to Console -> Management -> Devices.

  3. Clear all filters if necessary.

  4. Check whether all devices are displayed. In my case just 6 devices (only workstations) are displayed.

Expected behavior

All registered devices should be displayed when no particular location is selected.

Screenshots

This is how my filters look like with Location blank.

изображение

Desktop (please complete the following information)

  • OS: Windows 10 1909 (18363.752)

  • Browser: Firefox

  • Version: 74.0.1

@gdsotirov gdsotirov added bug Undesired behaviour unverified Some days we don't have a clue labels Apr 7, 2020
@gdsotirov
Copy link
Contributor Author

gdsotirov commented Apr 7, 2020

Some more information. This seems to be incorrect NULL value handling, because the 6 devices displayed all had value '' (empty string) in the database. After updating all devices with NULL to '' with the following query

UPDATE cacti.`host`
   SET location = ''
 WHERE location IS NULL
    OR location = 'None';

they are now listed in Console -> Management -> Devices. And the blank value in the filter is gone.

изображение

@netniV
Copy link
Member

netniV commented Apr 7, 2020

Not sure why your devices would have null in the location column. Are these on a secondary data collector or the main poller?

@rprofijt
Copy link

rprofijt commented Apr 7, 2020

Same issue here, no remote data pollers, just a single Cacti.

@netniV
Copy link
Member

netniV commented Apr 7, 2020

OK, so it's not a sync issue, something more fundemental. Let me take a look at my own stuff. In the meantime, can you provide a CREATE TABLE command so I can see how it's defined for you guys?

show create table `host`\G

@rprofijt
Copy link

rprofijt commented Apr 7, 2020

Create Table: CREATE TABLE `host` (
  `id` mediumint(8) unsigned NOT NULL AUTO_INCREMENT,
  `poller_id` int(10) unsigned NOT NULL DEFAULT '1',
  `site_id` int(10) unsigned NOT NULL DEFAULT '1',
  `host_template_id` mediumint(8) unsigned NOT NULL DEFAULT '0',
  `description` varchar(150) NOT NULL DEFAULT '',
  `hostname` varchar(100) DEFAULT '',
  `location` varchar(40) DEFAULT NULL,
  `notes` text,
  `external_id` varchar(40) DEFAULT NULL,
  `snmp_community` varchar(100) DEFAULT NULL,
  `snmp_version` tinyint(1) unsigned NOT NULL DEFAULT '1',
  `snmp_username` varchar(50) DEFAULT NULL,
  `snmp_password` varchar(50) DEFAULT NULL,
  `snmp_auth_protocol` char(6) DEFAULT '',
  `snmp_priv_passphrase` varchar(200) DEFAULT '',
  `snmp_priv_protocol` char(6) DEFAULT '',
  `snmp_context` varchar(64) DEFAULT '',
  `snmp_engine_id` varchar(64) DEFAULT '',
  `snmp_port` mediumint(5) unsigned NOT NULL DEFAULT '161',
  `snmp_timeout` mediumint(8) unsigned NOT NULL DEFAULT '500',
  `snmp_sysDescr` varchar(300) NOT NULL DEFAULT '',
  `snmp_sysObjectID` varchar(128) NOT NULL DEFAULT '',
  `snmp_sysUpTimeInstance` int(10) unsigned NOT NULL DEFAULT '0',
  `snmp_sysContact` varchar(300) NOT NULL DEFAULT '',
  `snmp_sysName` varchar(300) NOT NULL DEFAULT '',
  `snmp_sysLocation` varchar(300) NOT NULL DEFAULT '',
  `availability_method` smallint(5) unsigned NOT NULL DEFAULT '2',
  `ping_method` smallint(5) unsigned DEFAULT '0',
  `ping_port` int(12) unsigned DEFAULT '0',
  `ping_timeout` int(12) unsigned DEFAULT '500',
  `ping_retries` int(12) unsigned DEFAULT '2',
  `max_oids` int(12) unsigned DEFAULT '10',
  `device_threads` tinyint(2) unsigned NOT NULL DEFAULT '1',
  `deleted` char(3) NOT NULL DEFAULT '',
  `disabled` char(2) DEFAULT '',
  `thold_send_email` int(10) NOT NULL DEFAULT '1',
  `thold_host_email` int(10) NOT NULL,
  `status` tinyint(2) NOT NULL DEFAULT '0',
  `status_event_count` mediumint(8) unsigned NOT NULL DEFAULT '0',
  `status_fail_date` timestamp NOT NULL DEFAULT '0000-00-00 00:00:00',
  `status_rec_date` timestamp NOT NULL DEFAULT '0000-00-00 00:00:00',
  `status_last_error` varchar(255) DEFAULT NULL,
  `min_time` decimal(10,5) DEFAULT '9.99999',
  `max_time` decimal(10,5) DEFAULT '0.00000',
  `cur_time` decimal(10,5) DEFAULT '0.00000',
  `avg_time` decimal(10,5) DEFAULT '0.00000',
  `polling_time` double DEFAULT '0',
  `total_polls` int(12) unsigned DEFAULT '0',
  `failed_polls` int(12) unsigned DEFAULT '0',
  `availability` decimal(8,5) NOT NULL DEFAULT '100.00000',
  `last_updated` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,
  PRIMARY KEY (`id`),
  KEY `disabled` (`disabled`),
  KEY `external_id` (`external_id`),
  KEY `hostname` (`hostname`),
  KEY `status` (`status`),
  KEY `poller_id_disabled` (`poller_id`,`disabled`),
  KEY `poller_id_last_updated` (`poller_id`,`last_updated`),
  KEY `poller_id` (`poller_id`),
  KEY `site_id` (`site_id`),
  KEY `site_id_location` (`site_id`,`location`)
) ENGINE=InnoDB AUTO_INCREMENT=671 DEFAULT CHARSET=latin1

@netniV
Copy link
Member

netniV commented Apr 7, 2020

On my personal system, I have no devices with a null location, but my work has 110 with null, and 84 without. However the device list shows on 64 so something is clearly not quite right.

Also, your table does not look the same as mine:

@@ -4,7 +4,7 @@ Create Table: CREATE TABLE `host` (
   `site_id` int(10) unsigned NOT NULL DEFAULT '1',
   `host_template_id` mediumint(8) unsigned NOT NULL DEFAULT '0',
   `description` varchar(150) NOT NULL DEFAULT '',
-  `hostname` varchar(100) DEFAULT NULL,
+  `hostname` varchar(100) DEFAULT '',
   `location` varchar(40) DEFAULT NULL,
   `notes` text,
   `external_id` varchar(40) DEFAULT NULL,
@@ -25,22 +25,22 @@ Create Table: CREATE TABLE `host` (
   `snmp_sysContact` varchar(300) NOT NULL DEFAULT '',
   `snmp_sysName` varchar(300) NOT NULL DEFAULT '',
   `snmp_sysLocation` varchar(300) NOT NULL DEFAULT '',
-  `availability_method` smallint(5) unsigned NOT NULL DEFAULT '1',
+  `availability_method` smallint(5) unsigned NOT NULL DEFAULT '2',
   `ping_method` smallint(5) unsigned DEFAULT '0',
   `ping_port` int(12) unsigned DEFAULT '0',
   `ping_timeout` int(12) unsigned DEFAULT '500',
   `ping_retries` int(12) unsigned DEFAULT '2',
   `max_oids` int(12) unsigned DEFAULT '10',
   `device_threads` tinyint(2) unsigned NOT NULL DEFAULT '1',
-  `deleted` char(2) DEFAULT '',
-  `disabled` char(2) DEFAULT NULL,
+  `deleted` char(3) NOT NULL DEFAULT '',
+  `disabled` char(2) DEFAULT '',
   `thold_send_email` int(10) NOT NULL DEFAULT '1',
-  `thold_host_email` int(10) DEFAULT NULL,
+  `thold_host_email` int(10) NOT NULL,
   `status` tinyint(2) NOT NULL DEFAULT '0',
   `status_event_count` mediumint(8) unsigned NOT NULL DEFAULT '0',
   `status_fail_date` timestamp NOT NULL DEFAULT '0000-00-00 00:00:00',
   `status_rec_date` timestamp NOT NULL DEFAULT '0000-00-00 00:00:00',
-  `status_last_error` varchar(255) DEFAULT '',
+  `status_last_error` varchar(255) DEFAULT NULL,
   `min_time` decimal(10,5) DEFAULT '9.99999',
   `max_time` decimal(10,5) DEFAULT '0.00000',
   `cur_time` decimal(10,5) DEFAULT '0.00000',
@@ -51,12 +51,13 @@ Create Table: CREATE TABLE `host` (
   `availability` decimal(8,5) NOT NULL DEFAULT '100.00000',
   `last_updated` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,
   PRIMARY KEY (`id`),
-  KEY `poller_id_disabled` (`poller_id`,`disabled`),
-  KEY `site_id` (`site_id`),
-  KEY `external_id` (`external_id`),
   KEY `disabled` (`disabled`),
-  KEY `status` (`status`),
-  KEY `site_id_location` (`site_id`,`location`),
+  KEY `external_id` (`external_id`),
   KEY `hostname` (`hostname`),
+  KEY `status` (`status`),
+  KEY `poller_id_disabled` (`poller_id`,`disabled`),
-  KEY `poller_id_last_updated` (`poller_id`,`last_updated`)
+  KEY `poller_id_last_updated` (`poller_id`,`last_updated`),
+  KEY `poller_id` (`poller_id`),
+  KEY `site_id` (`site_id`),
+  KEY `site_id_location` (`site_id`,`location`)
-) ENGINE=InnoDB AUTO_INCREMENT=4 DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci
+) ENGINE=InnoDB AUTO_INCREMENT=671 DEFAULT CHARSET=latin1

Some of this is due to ordering, but there are other subtle differences like the default being blank instead of null in a few places.

@netniV
Copy link
Member

netniV commented Apr 7, 2020

I'm continuing to investigate using my work system.

@netniV
Copy link
Member

netniV commented Apr 7, 2020

OK, think I've found this. Patch coming.

@netniV
Copy link
Member

netniV commented Apr 7, 2020

Please give the above patch a try.

@netniV netniV self-assigned this Apr 7, 2020
@netniV netniV added this to the 1.2.12 milestone Apr 7, 2020
@netniV netniV changed the title Most devices not listed after upgrade to 1.2.11 Hosts are being incorrectly filtered when no location filter is set Apr 7, 2020
@netniV
Copy link
Member

netniV commented Apr 7, 2020

With my above patch/commit, I now get the full 174 devices I expect.

@gregnetau
Copy link

Confirming i just experienced this same issue with a 1.2.11 upgrade, although I also ran into an issue with the upgrade getting stuck at 44%.

@netniV
Copy link
Member

netniV commented Apr 7, 2020

@gregnetau try to apply the commit as that should resolve things for you.

@gregnetau
Copy link

Some more information. This seems to be incorrect NULL value handling, because the 6 devices displayed all had value '' (empty string) in the database. After updating all devices with NULL to '' with the following query

UPDATE cacti.`host`
   SET location = ''
 WHERE location IS NULL
    OR location = 'None';

they are now listed in Console -> Management -> Devices. And the blank value in the filter is gone.

изображение

@netniV this SQL query fixed it for me - so i assume your patch will do the same. It did not however resolve my upgrade being stuck at 44%.

When i navigated away from the install to the Cacti instance it seemed to resolve itself.

@netniV netniV added confirmed Bug is confirm by dev team gui UI related issue resolved A fixed issue and removed unverified Some days we don't have a clue labels Apr 7, 2020
@netniV
Copy link
Member

netniV commented Apr 7, 2020

Please log the install issue as a separate one if we need to persue it, but as you've skipped past that issue, I am not sure there's much we can do on it right now.

@netniV netniV closed this as completed Apr 7, 2020
@gdsotirov
Copy link
Contributor Author

Not sure why your devices would have null in the location column. Are these on a secondary data collector or the main poller?

@netniV Main poller. I don't use location, but this was the data I had.

@gdsotirov
Copy link
Contributor Author

... can you provide a CREATE TABLE command so I can see how it's defined for you guys?

@netniV This is how location is defined on my system:

  `location` varchar(40) DEFAULT NULL,

So it seems normal for me that NULL values exist.

@gdsotirov
Copy link
Contributor Author

Wow... until I was fixing my instance this one got investigated and fixed. Thanks @netniV !

The patch seems fine to me, but I also applied it on my instance to try it. I now get a full list of devices, but by default I have again the blank option (for the NULLs). Should not this be "Undefined" assuming NULL and empty string should be synonyms of "Undefined"? I'm not sure how the contents of this list are generated.

@gdsotirov
Copy link
Contributor Author

@netniV There is still problem with the filter. I tested with the following values in column hosts.location - NULL, empty string and 'None' (which I had before BTW). The filter in this case displays - All, blank value, None and Undefined.

изображение

With the blank value selected by default. Also whatever is selected, the filter returns to the blank value and the list still contains just the devices having NULL or empty string as value for location, which makes it useless. I'm practically unable to filter by the value 'None' for example or select All.

@gdsotirov
Copy link
Contributor Author

The problem is that the location filter's value is not passed in the URL, which explains why it's not preserved.

@netniV
Copy link
Member

netniV commented Apr 7, 2020

I believe that's a secondary issue, one that we should log separately from the missing devices, but lets try to get that fixed too 👍

@gdsotirov
Copy link
Contributor Author

gdsotirov commented Apr 7, 2020

@netniV I think I found the problem now after debugging in Firefox. On my installation I also have Monitor plugin version 2.3.4 installed and enabled. This plugin seems to override applyFilter function (see at line 110) from host.php (see at line 1429) effectively breaking the new location filter. This has been added with commit 26e8f90 by @cigamit to enable criticality filter on devices page. I added location parameter into this function and location filter now works for me. Perhaps you have to discuss how this should be resolved, because I'm not that familiar with Cacti sources, but seems plugins need to setup filters as well.

@gdsotirov
Copy link
Contributor Author

These are the current differences between the two functions:

diff -u cacti_host_php monitor_setup_php
--- cacti_host_php      2020-04-07 18:53:24.372823995 +0300
+++ monitor_setup_php   2020-04-07 18:54:12.913743825 +0300
@@ -1,12 +1,12 @@
-       function applyFilter() {
-               strURL  = 'host.php';
-               strURL += '?host_status=' + $('#host_status').val();
+       applyFilter = function() {
+               strURL  = 'host.php?host_status=' + $('#host_status').val();
                strURL += '&host_template_id=' + $('#host_template_id').val();
                strURL += '&site_id=' + $('#site_id').val();
+               strURL += '&criticality=' + $('#criticality').val();
                strURL += '&poller_id=' + $('#poller_id').val();
-               strURL += '&location=' + $('#location').val();
                strURL += '&rows=' + $('#rows').val();
                strURL += '&filter=' + $('#filter').val();
+               strURL += '&page=' + $('#page').val();
                strURL += '&header=false';
                loadPageNoHeader(strURL);
-       }
+       };

@gdsotirov
Copy link
Contributor Author

I just opened PR 129 for the Monitor plugin.

@vKnmnn
Copy link

vKnmnn commented Apr 28, 2020

coming from #3508, your patch brings back my 530 missing hosts. Thank you, Mark.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Undesired behaviour confirmed Bug is confirm by dev team gui UI related issue resolved A fixed issue
Projects
None yet
Development

No branches or pull requests

5 participants