ZABBIX BUGS AND ISSUES

trigger addDependencies() creates duplicates

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Fixed
  • Affects Version/s: 1.8.4
  • Fix Version/s: 2.0.4rc1, 2.1.0
  • Component/s: API (A)
  • Labels:
  • Environment:
    MySQL RDBMS using InnoDB engine for tables
  • Zabbix ID:
    RTD

Description

The API method trigger.addDependencies does not check whether dependencies exist prior to adding them, and there is no unique constraint across trigger_depends.triggerid_up and trigger_depends.triggerid_down to prevent the addition.

We had poorly written code that was unconditionally adding a dependency over and over, e.g.

# request:
{
"jsonrpc":"2.0",
"method":"trigger.addDependencies",
"params":[
    {
        "triggerid": "100100000065000",
        "dependsOnTriggerid": "100100000064000"
    },
],
"auth":"038e1d7b1735c6a5436ee9eae095879e",
"id":2
}

The problem is, when we checked what this was doing, through the API, it looked like trigger.addDependencies was being very clever, and doing nothing if the dependency already existed:

# request:
{
"jsonrpc":"2.0",
"method":"trigger.get",
"params":{
    "filter":{
        "description": ["Sample trigger 1"],
    },
    "include_dependencies": "refer"
},
"auth":"6f38cddc44cfbb6c1bd186f9a220b5a0",
"id":2
}

No matter how many times we added the same dependency with trigger.addDependencies, trigger.get would show us the dependency only once.

However, each call to trigger.addDependencies added another row to the trigger_depends table. By the time we noticed what was happening, we had over half a million rows in the trigger_depends table, the increasing every half an hour when our script was run from cron.

We have subsequently modified our code to "include_dependencies" in the trigger.get request, and only trigger.addDependencies if the dependencies we want are missing. This reduces the likelihood of duplicate rows in trigger_depends table considerably.

Possible solutions that occur to me include:

1) Make trigger.addDependencies() fail when it receives a request to add an already existing dependency.
2) Throw a unique constraint across trigger_depends.triggerid_up and trigger_depends.triggerid_down and just let the mysql exception explode upward.
3) Make trigger.addDependencies() silently ignore a request to add an already existing dependency. Perhaps log a warning.

Issue Links

Activity

Hide
Alexey Fukalov added a comment - - edited

(1)
Not sure but it looks like than instead of select in loop it can be done with one select with grouping and count. If it's not or too slow, it should be moved to checkDependencies method as there already exists same loop with same SELECT.

Oleg Egorov RESOLVED

Alexey Fukalov some code corrections needed as we discussed.

Oleg Egorov RESOLVED

Alexey Fukalov CLSOED

Show
Alexey Fukalov added a comment - - edited (1) Not sure but it looks like than instead of select in loop it can be done with one select with grouping and count. If it's not or too slow, it should be moved to checkDependencies method as there already exists same loop with same SELECT. Oleg Egorov RESOLVED Alexey Fukalov some code corrections needed as we discussed. Oleg Egorov RESOLVED Alexey Fukalov CLSOED
Hide
Alexey Fukalov added a comment - - edited

(2)
Code style is not correct in some places:

if($DplTriggers){
$UnqTriggers  = array_unique($trigger['dependencies']);

Oleg Egorov
REW 30386
RESOLVED

Alexey Fukalov CLSOED

Show
Alexey Fukalov added a comment - - edited (2) Code style is not correct in some places:
if($DplTriggers){
$UnqTriggers  = array_unique($trigger['dependencies']);
Oleg Egorov REW 30386 RESOLVED Alexey Fukalov CLSOED
Hide
Alexey Fukalov added a comment - - edited

(3)
When try to add duplicate dependency error is thrown:
SQL statement execution has failed \"INSERT INTO trigger_depends (triggerid_down,triggerid_up,triggerdepid) VALUES ('13645','13664','41')

Oleg Egorov RESOLVED

Alexey Fukalov seems there is still needed separate select as discussed.

Oleg Egorov RESOLVED

Show
Alexey Fukalov added a comment - - edited (3) When try to add duplicate dependency error is thrown: SQL statement execution has failed \"INSERT INTO trigger_depends (triggerid_down,triggerid_up,triggerdepid) VALUES ('13645','13664','41') Oleg Egorov RESOLVED Alexey Fukalov seems there is still needed separate select as discussed. Oleg Egorov RESOLVED
Hide
Alexey Fukalov added a comment - - edited

Alexey Fukalov as discussed, select logic and error message should be changed.

Oleg Egorov RESOLVED IN svn://svn.zabbix.com/branches/dev/ZBX-3920 r30669

Alexey Fukalov seems it doesn't catch db duplicates, show message about insert sql failure.

Alexey Fukalov Please review my changes in r30671 ,can be merged if everything is ok.

Oleg Egorov CLOSED

Show
Alexey Fukalov added a comment - - edited Alexey Fukalov as discussed, select logic and error message should be changed. Oleg Egorov RESOLVED IN svn://svn.zabbix.com/branches/dev/ZBX-3920 r30669 Alexey Fukalov seems it doesn't catch db duplicates, show message about insert sql failure. Alexey Fukalov Please review my changes in r30671 ,can be merged if everything is ok. Oleg Egorov CLOSED
Hide
Oleg Egorov added a comment - - edited

Fixed in pre-2.0.4 r30672 and pre-2.1.0 (trunk) r30673.

Show
Oleg Egorov added a comment - - edited Fixed in pre-2.0.4 r30672 and pre-2.1.0 (trunk) r30673.

People

Vote (2)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: