[Asterisk-code-review] res odbc: Use negative connection cache for all connections (asterisk[master])

[Joshua Colp]

Joshua Colp has posted comments on this change.

Change subject: res_odbc: Use negative connection cache for all connections
......................................................................


Patch Set 4: Code-Review-1

(2 comments)

https://gerrit.asterisk.org/#/c/182/4//COMMIT_MSG
Commit Message:

Line 9: Reported as a bug. When server is marked as unconectable, we should
      : not try to conect in any way.
I don't think this describes well enough what is going on. A better description would be:

"Apply the negative connection cache setting to all connections, even those that are not pooled. This ensures that the connection will not be re-established before the negative connection cache time is met."


https://gerrit.asterisk.org/#/c/182/4/res/res_odbc.c
File res/res_odbc.c:

Line 1250: 	long int negative_cache_expiration = class->last_negative_connect.tv_sec + class->negative_connection_cache.tv_sec;
         : 	if (time(NULL) < negative_cache_expiration) {
         : 		ast_log(LOG_WARNING, "Not connecting to %s. Negative connection cache for %ld seconds\n", name, negative_cache_expiration - time(NULL));
         : 		ao2_ref(class, -1);
         : 		return NULL;
         : 	}
Moving this here breaks the pooling case.

If a valid connection is available you now exit early, so it won't get used.

As well variables need to be declared at the start.


-- 
To view, visit https://gerrit.asterisk.org/182
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I431cc2e8584ab0b6908b3523d0a0e18c9a527271
Gerrit-PatchSet: 4
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Martin Tomec <tomec.martin at gmail.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Martin Tomec <tomec.martin at gmail.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Walter Doekes <walter+asterisk at wjd.nu>
Gerrit-HasComments: Yes