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

[Sean Bright]

Attention is currently required from: Sean Bright, Dennis.

Sean Bright has uploaded a new patch set (#4) to the change originally created by Dennis. ( https://gerrit.asterisk.org/c/asterisk/+/14698 )

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 spliting 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, 892 insertions(+), 672 deletions(-)


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

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: Id85d81add33096f8282d212daf239f2fc845d783
Gerrit-Change-Number: 14698
Gerrit-PatchSet: 4
Gerrit-Owner: Dennis <dennis.buteyn at xorcom.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Kevin Harwell <default.enum at gmail.com>
Gerrit-Reviewer: Sean Bright <sean at seanbright.com>
Gerrit-CC: Joshua Colp <jcolp at sangoma.com>
Gerrit-Attention: Sean Bright <sean at seanbright.com>
Gerrit-Attention: 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/96f574f2/attachment-0001.html>