[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:
Duplicate

 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:

  • 2.2.17rc1 r65200
  • 3.0.8rc1 r65201
  • 3.2.4rc1 r65202
  • 3.3.0 r65203
Generated at Fri Mar 29 12:32:21 EET 2024 using Jira 9.12.4#9120004-sha1:625303b708afdb767e17cb2838290c41888e9ff0.