[Asterisk-code-review] cel_odbc: Rewritten most of cel_odbc (asterisk[18])

[Sean Bright]

Hello Friendly Automation, 

I'd like you to reexamine a change. Please visit

    https://gerrit.asterisk.org/c/asterisk/+/19534

to look at the new patch set (#3).

Change subject: cel_odbc: Rewritten most of cel_odbc
......................................................................

cel_odbc: Rewritten most of cel_odbc

We found that if a datasource is unavailable when cel_odbc.conf is
processed, the table is skipped and no records will ever be inserted
into the skipped table.

While splitting code apart, I found lots of interesting problems
hidden inside this file and ended up rewriting most of it. Trying to
keep changes minimal became a near impossible task because of the
amount of collisions and rewrites.

The first problem I encountered was malloc tricks and pointer hacks,
stuffing null-terminated strings below structures. As these structures
were only created when cel_odbc.conf is parsed, the loss of
maintainability far outweighs any performance gained from doing so.

Next, I split up all the massive functions best I could. Each function
now has its own exact purpose. Some functions were renamed during this
process to better describe what they do.

I also renamed many variables to what they really are/contained.
Redundant logging of allocation failures was removed as ast_calloc
already logs them. Functions were added to allocate and initialize
every structure with proper default values. Lots of temporary
variables were removed, avoiding extra string copies.

Arbitrary length limits on strings have been mostly removed.
Configuration file is now traversed instead of scanned repeatedly.

The SQL_ERROR return code from SQLFetch is now handled properly,
causing the table to be skipped instead of trying to perform partial
inserts into it.

Optimizing odbc_log() was fairly interesting and while I haven't
benchmarked should be fairly fruitful. The original algorithm
constructed completely new SQL for every table on every event,
stringifying values in the process and performing
events*tables*columns*20-ish string comparisons. The new algorithm
generates static queries once tables columns are known. Relative
addresses are generated for all parameters, to be used later when
binding parameters. By binding parameters instead of stringifying them
we allow the driver to decide on the most optimal conversion.

All identifiers have been wrapped with delimiters as specified by
driver, which should close any tickets related to column names and
reserved words.  Because we no longer perform type checks on all
columns, I was able to discard lots of extra data from the columns
struct.

Lastly, I tried to make verbose logs a bit more clear and useful.

Hopefully I was able to re-implement the "static", "filter" and
"alias" features properly. Documentation on how they actually function
was somewhat light.

This module now works as follows:

First, it reads the configuration file and stores all relevant bits in
structures for later. Once this is done, it will attempt to retrieve
column names for every table mentioned in the config file, construct a
query for these tables and pre-bind any parameters.

For any tables that were inaccessible at config time, a manager thread
will go through the list of tables every 10 seconds and attempts to
construct queries for any tables that this module failed to connect to
previously.

ASTERISK-29012 #close

Change-Id: Id85d81add33096f8282d212daf239f2fc845d783
---
M cel/cel_odbc.c
1 file changed, 863 insertions(+), 675 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/34/19534/3
-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/19534
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 18
Gerrit-Change-Id: Id85d81add33096f8282d212daf239f2fc845d783
Gerrit-Change-Number: 19534
Gerrit-PatchSet: 3
Gerrit-Owner: Sean Bright <sean at seanbright.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-CC: Dennis <dennis.buteyn at xorcom.com>
Gerrit-MessageType: newpatchset
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20221104/4553cfa2/attachment-0001.html>