[ZBX-11587] Z3005 error in commit leads to ignore next queries outside from transaction Created: 2016 Dec 12 Updated: 2017 May 30 Resolved: 2017 Jan 26 |
|
Status: | Closed |
Project: | ZABBIX BUGS AND ISSUES |
Component/s: | Proxy (P), Server (S) |
Affects Version/s: | 2.2.16, 3.0.6, 3.2.2 |
Fix Version/s: | 2.2.17rc1, 3.0.8rc1, 3.2.4rc1, 3.4.0alpha1 |
Type: | Incident report | Priority: | Critical |
Reporter: | Alexey Pustovalov | Assignee: | Unassigned |
Resolution: | Fixed | Votes: | 0 |
Labels: | deadlock | ||
Remaining Estimate: | Not Specified | ||
Time Spent: | Not Specified | ||
Original Estimate: | Not Specified |
Issue Links: |
|
Description |
Currently if Z3005 issue ZBX_DB_FAIL happens in db_commit and next queries are not in transaction, all of them will be ignored: 20270:20161212:033431.652 [Z3005] query failed: [1213] Deadlock found when trying to get lock; try restarting transaction [commit;] |
Comments |
Comment by Viktors Tjarve [ 2016 Dec 20 ] |
In src/libs/zbxdb/dc.c global variable txn_error cannot be set to 1 if ZBX_DB_FAIL is set during DBcommit(). In such case also zbx_db_rollback() has to be called. Fixed in development branch svn://svn.zabbix.com/branches/dev/ZBX-11587 |
Comment by Aleksandrs Saveljevs [ 2016 Dec 21 ] |
(1) If we are now protecting against commit failures and doing a rollback instead, then should we check the return value of OCITransCommit() for Oracle? viktors.tjarve RESOLVED in r64789. s.paskevics CLOSED. |
Comment by Aleksandrs Saveljevs [ 2016 Dec 21 ] |
(2) The placement of the following code in zbx_db_commit() is suspicious: if (ZBX_DB_FAIL == rc) return zbx_db_rollback(); In particular, it seems that for SQLite3 we will end up calling zbx_mutex_unlock(&sqlite_access) twice: once in zbx_db_commit() and once in zbx_db_rollback(). Also, if we leave it there, then it probably deserves a log entry similar to "commit called on failed transaction, doing a rollback instead" above. viktors.tjarve RESOLVED in r64791. s.paskevics CLOSED. |
Comment by Aleksandrs Saveljevs [ 2016 Dec 21 ] |
(3) The following code in zbx_db_vexecute() is creepy, because it compares against a whole literal SQL query to determine whether we are doing a commit or not: if (0 != strcmp("commit;", sql)) txn_error = 1; How about trying a different approach: for instance, reverting the code in zbx_db_vexecute() to how it was before, but instead checking the value of txn_error in zbx_db_commit() after executing the commit; statement and resetting if need be? viktors.tjarve RESOLVED in r64790. s.paskevics CLOSED. |
Comment by Sergejs Paskevics [ 2017 Jan 02 ] |
(4) These lines can be merged in a single expression: #elif defined(HAVE_MYSQL) || defined(HAVE_POSTGRESQL) rc = zbx_db_execute("%s", "commit;"); and #elif defined(HAVE_SQLITE3) rc = zbx_db_execute("%s", "commit;"); viktors.tjarve RESOLVED in r64867. s.paskevics CLOSED. |
Comment by Sergejs Paskevics [ 2017 Jan 02 ] |
(5) There is no need to initialize err variable: sword err = OCI_SUCCESS; viktors.tjarve RESOLVED in r64867. s.paskevics CLOSED. |
Comment by Sergejs Paskevics [ 2017 Jan 02 ] |
(6) Compilation errors: gcc -DHAVE_CONFIG_H -I. -I../../../include -g -O2 -I/u01/app/oracle/product/11.2.0/xe/rdbms/public -I/u01/app/oracle/product/11.2.0/xe/rdbms/demo -I/usr/include/libxml2 -I/usr/local/include -I/usr/lib/x86_64-linux-gnu/perl/5.22/CORE -I. -I/usr/local/include -MT db.o -MD -MP -MF $depbase.Tpo -c -o db.o db.c &&\ mv -f $depbase.Tpo $depbase.Po db.c: In function ‘zbx_db_commit’: db.c:752:3: error: expected ‘)’ before ‘rc’ rc = OCI_handle_sql_error(ERR_Z3005, err, sql); ^ db.c:772:1: error: expected ‘)’ before ‘}’ token } ^ db.c:772:1: error: expected expression before ‘}’ token Makefile:399: recipe for target 'db.o' failed make[3]: *** [db.o] Error 1 make[3]: Leaving directory '/home/spaskevics/projects/zabbix/ZBX-11587/src/libs/zbxdb' viktors.tjarve RESOLVED in r64867. s.paskevics CLOSED. |
Comment by Sergejs Paskevics [ 2017 Jan 06 ] |
(7) Function zbx_db_rollback must return correct error code. Now this function always returns ZBX_DB_OK for oracle version. viktors.tjarve RESOLVED in r64990. s.paskevics CLOSED. |
Comment by Sergejs Paskevics [ 2017 Jan 06 ] |
(8) This condition has never been executed for oracle version: if (ZBX_DB_FAIL == rc && 1 == txn_error) { zabbix_log(LOG_LEVEL_DEBUG, "commit called on failed transaction, doing a rollback instead"); txn_error = 0; return zbx_db_rollback(); } viktors.tjarve RESOLVED in r65054 and r65134. s.paskevics CLOSED. |
Comment by Sergejs Paskevics [ 2017 Jan 19 ] |
Successfully tested. |
Comment by Viktors Tjarve [ 2017 Jan 20 ] |
Released in:
|