ZABBIX BUGS AND ISSUES
  1. ZABBIX BUGS AND ISSUES
  2. ZBX-3920

trigger addDependencies() creates duplicates

    Details

    • Type: Bug Bug
    • Status: 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

      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.

      1. 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:

      1. 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

            • Assignee:
              Unassigned
              Reporter:
              Sheldon Hearn
            • Votes:
              2 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: