GSQL: use lazy prepared statements #3937

Merged
merged 14 commits into from Jun 28, 2016

Projects

None yet

4 participants

@cmouse
Contributor
cmouse commented Jun 6, 2016

Change GSQL drives to defer statement initialization to happen when they are actually needed.

@cmouse
Contributor
cmouse commented Jun 6, 2016

@zeha, @mind04, @Habbie please review if you have a spare moment

@cmouse cmouse changed the title from GSQL: use lazy prepared statements to WIP: GSQL: use lazy prepared statements Jun 6, 2016
@cmouse
Contributor
cmouse commented Jun 6, 2016 edited
  • code review
  • external testing (if we can find someone)
  • oracle testing
  • correct use of exceptions (fatal, non-fatal distinction)
  • correct use of releaseStatement (only in non-fatal cases)
@cmouse cmouse changed the title from WIP: GSQL: use lazy prepared statements to GSQL: use lazy prepared statements Jun 6, 2016
@zeha zeha commented on the diff Jun 6, 2016
modules/gmysqlbackend/smysql.cc
}
SSqlStatement* bind(const string& name, bool value) {
- if (d_paridx >= d_parnum)
+ prepareStatement();
+ if (d_paridx >= d_parnum) {
+ releaseStatement();
@zeha
zeha Jun 6, 2016 Collaborator

shouldnt it be enough to just let the destructor do this?

@cmouse
cmouse Jun 6, 2016 Contributor

we are not going to destroy the statement in the future if I've understood correctly, but yes, in this case it probably should be and then dtor would be handling it.

I would welcome any input on how to do signalling about when reprepare won't save you and when it does.

@cmouse
cmouse Jun 6, 2016 Contributor

@ahupowerdns told not to change this.

@zeha zeha commented on an outdated diff Jun 6, 2016
modules/goraclebackend/soracle.cc
+
+ void initStatement() {
+ if (d_query.size()==0) return;
+ if (d_prepared) return;
+
+ if (OCIHandleAlloc(d_ctx, (dvoid**)&d_err, OCI_HTYPE_ERROR, 0, NULL))
+ throw SSqlException("Cannot allocate statement error handle");
+
+ if (d_release_stmt) {
+ if (OCIStmtPrepare2(d_svcctx, &d_stmt, d_err, (text*)d_query.c_str(), d_query.size(),
+ NULL, 0, OCI_NTV_SYNTAX, OCI_DEFAULT) != OCI_SUCCESS) {
+ // failed.
+ throw SSqlException("Cannot prepare statement (1): " + d_query + string(": ") + OCIErrStr());
+ }
+ d_init = true;
+ } else d_init = false;
@zeha
zeha Jun 6, 2016 Collaborator

please don't have a single line else branch with no braces after a multi-line if branch (for readability)

@zeha zeha commented on an outdated diff Jun 6, 2016
modules/goraclebackend/soracle.cc
@@ -467,7 +488,7 @@ SOracle::~SOracle()
}
void SOracle::startTransaction() {
- std::string cmd = "SET TRANSACTION NAME '" + std::to_string(s_txid++) + "'";
+ std::string cmd = "SET TRANSACTION NAME 'tx_" + std::to_string(s_txid++) + "'";
@zeha
zeha Jun 6, 2016 Collaborator

maybe pdns_ instead of tx_?

@j0ju
j0ju commented Jun 7, 2016 edited

This seems to fix #3933. From the comments I guess it should simply reconnect after a database backend fails and comes back up. And then it should redo the statement preparations as needed.
So I tested taking down the mysql database and bring it up again.

From the logs:

   Jun 07 10:48:00 example.com systemd[1]: Starting PowerDNS Authoritative Server ext...
   Jun 07 10:48:00 example.com pdns_server[20016]: Jun 07 10:48:00 UDP server bound to 203.0.113.1:55555
   Jun 07 10:48:00 example.com pdns_server[20016]: Jun 07 10:48:00 UDPv6 server bound to [2001:db8:201:43aa::1:2]:55555
   Jun 07 10:48:00 example.com pdns_server[20016]: Jun 07 10:48:00 TCP server bound to 203.0.113.1:55555
   Jun 07 10:48:00 example.com pdns_server[20016]: Jun 07 10:48:00 TCPv6 server bound to [2001:db8:201:43aa::1:2]:55555
   Jun 07 10:48:00 example.com pdns_server[20016]: Jun 07 10:48:00 Creating backend connection for TCP
   Jun 07 10:48:00 example.com systemd[1]: Started PowerDNS Authoritative Server ext.
   Jun 07 10:48:29 example.com pdns_server[20016]: Jun 07 10:48:29 AXFR of domain 'example.com.' initiated by 203.0.113.1
   Jun 07 10:48:29 example.com pdns_server[20016]: Jun 07 10:48:29 AXFR of domain 'example.com.' to 203.0.113.1 finished
   Jun 07 10:56:15 example.com pdns_server[20016]: Jun 07 10:56:15 AXFR of domain 'example.com.' initiated by 203.0.113.1
   Jun 07 10:56:15 example.com pdns_server[20016]: Jun 07 10:56:15 TCP nameserver had error, cycling backend: GSQLBackend lookup query:Could not execute mysql statement: SELECT content,ttl,prio,type,domain_id,disabled,name,auth FROM reco
   Jun 07 10:56:16 example.com pdns_server[20016]: Jun 07 10:56:16 AXFR of domain 'example.com.' initiated by 203.0.113.1
   Jun 07 10:56:16 example.com pdns_server[20016]: Jun 07 10:56:16 TCP server is without backend connections in doAXFR, launching
   Jun 07 10:56:16 example.com pdns_server[20016]: Jun 07 10:56:16 gmysql Connection failed: Unable to connect to database: Can't connect to MySQL server on '10.23.43.1' (111)
   Jun 07 10:56:16 example.com pdns_server[20016]: Jun 07 10:56:16 Caught an exception instantiating a backend: Unable to launch gmysql connection: Unable to connect to database: Can't connect to MySQL server on '10.23.43.1' (111)
   Jun 07 10:56:16 example.com pdns_server[20016]: Jun 07 10:56:16 Cleaning up
   Jun 07 10:56:16 example.com systemd[1]: pdns@ext.service: main process exited, code=exited, status=1/FAILURE

From now on it starts over and over again:

   Jun 07 10:56:16 example.com systemd[1]: Unit pdns@ext.service entered failed state.
   Jun 07 10:56:16 example.com systemd[1]: pdns@ext.service holdoff time over, scheduling restart.
   Jun 07 10:56:16 example.com systemd[1]: Stopping PowerDNS Authoritative Server ext...
   Jun 07 10:56:16 example.com systemd[1]: Starting PowerDNS Authoritative Server ext...
   Jun 07 10:56:16 example.com pdns_server[22983]: Jun 07 10:56:16 UDP server bound to 203.0.113.1:55555
   Jun 07 10:56:16 example.com pdns_server[22983]: Jun 07 10:56:16 UDPv6 server bound to [2001:db8:201:43aa::1:2]:55555
   Jun 07 10:56:16 example.com pdns_server[22983]: Jun 07 10:56:16 TCP server bound to 203.0.113.1:55555
   Jun 07 10:56:16 example.com pdns_server[22983]: Jun 07 10:56:16 TCPv6 server bound to [2001:db8:201:43aa::1:2]:55555
   Jun 07 10:56:16 example.com pdns_server[22983]: Jun 07 10:56:16 Creating backend connection for TCP
   Jun 07 10:56:16 example.com pdns_server[22983]: Jun 07 10:56:16 gmysql Connection failed: Unable to connect to database: Can't connect to MySQL server on '10.23.43.1' (111)
   Jun 07 10:56:16 example.com pdns_server[22983]: Jun 07 10:56:16 Caught an exception instantiating a backend: Unable to launch gmysql connection: Unable to connect to database: Can't connect to MySQL server on '10.23.43.1' (111)
   Jun 07 10:56:16 example.com pdns_server[22983]: Jun 07 10:56:16 Cleaning up
   Jun 07 10:56:16 example.com systemd[1]: pdns@ext.service: main process exited, code=exited, status=1/FAILURE

... until the MariaDB is back ...

   Jun 07 10:56:45 example.com systemd[1]: pdns@ext.service: main process exited, code=exited, status=1/FAILURE
   Jun 07 10:56:45 example.com systemd[1]: Failed to start PowerDNS Authoritative Server ext.
   Jun 07 10:56:45 example.com systemd[1]: Unit pdns@ext.service entered failed state.
   Jun 07 10:56:45 example.com systemd[1]: pdns@ext.service holdoff time over, scheduling restart.
   Jun 07 10:56:45 example.com systemd[1]: Stopping PowerDNS Authoritative Server ext...
   Jun 07 10:56:45 example.com systemd[1]: Starting PowerDNS Authoritative Server ext...
   Jun 07 10:56:45 example.com pdns_server[24119]: Jun 07 10:56:45 UDP server bound to 203.0.113.1:55555
   Jun 07 10:56:45 example.com pdns_server[24119]: Jun 07 10:56:45 UDPv6 server bound to [2001:db8:201:43aa::1:2]:55555
   Jun 07 10:56:45 example.com pdns_server[24119]: Jun 07 10:56:45 TCP server bound to 203.0.113.1:55555
   Jun 07 10:56:45 example.com pdns_server[24119]: Jun 07 10:56:45 TCPv6 server bound to [2001:db8:201:43aa::1:2]:55555
   Jun 07 10:56:45 example.com pdns_server[24119]: Jun 07 10:56:45 Creating backend connection for TCP
   Jun 07 10:56:45 example.com systemd[1]: Started PowerDNS Authoritative Server ext.
@pieterlexis
Member

2a4ac82 mitigates the restart issue. We've decided that exit()'ing on instantiation failure is ok.

Furthermore, I think the final 2 checkboxes:

correct use of exceptions (fatal, non-fatal distinction)
correct use of releaseStatement (only in non-fatal cases)

Could be moved to a post-4.0.0 ticket

@cmouse
Contributor
cmouse commented Jun 8, 2016

Would be nice if someone could do final code review.

@pieterlexis pieterlexis merged commit c34a446 into PowerDNS:master Jun 28, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment