[asterisk-dev] [Code Review] Replace Berkeley DB astdb with SQLite3

[Terry Wilson]

> On 2011-06-14 11:04:38, Tilghman Lesher wrote:
> > /trunk/main/db.c, line 991
> > <https://reviewboard.asterisk.org/r/1269/diff/1/?file=16882#file16882line991>
> >
> >     As this interface is marked experimental, if we're using a system library and not including sqlite3 directly within the source, this constant will need to be verified in the current system library.
> 
> Terry Wilson wrote:
>     Actually, I should be able to take it out entirely as I am overriding with the NOMUTEX when I open the database anyway.
> 
> Tilghman Lesher wrote:
>     I'm wondering if it might even be better to use the _MULTITHREAD value and let SQLite decide, internally, when a mutex is warranted.
>     
>     Of course, given that the API is marked experimental, it's difficult to justify the danger of that API disappearing unless we included the SQLite source directly.

It isn't. :-)

That was how I designed it the first time I wrote it, with thread local db connections and let SQLite handle everything itself. Unfortunately, to get decent performance, you have to wrap multiple queries into a single transaction, otherwise it is over 150x slower. It is much easier to just do the locking ourselves in that case. It also sped up a little bit doing the locking ourselves.


> On 2011-06-14 11:04:38, Tilghman Lesher wrote:
> > /trunk/main/db.c, line 132
> > <https://reviewboard.asterisk.org/r/1269/diff/1/?file=16882#file16882line132>
> >
> >     This statement should have an additional '/' between the wildcard and the argument specifier.
> 
> Terry Wilson wrote:
>     I don't think so. The value that is queried for has the '/' in it. I tried to keep the logic as close to the original as possible.
>     
>     
>         if (a->argc == 3) {
>             /* Key only */
>             snprintf(suffix, sizeof(suffix), "/%s", a->argv[2]);
>         } else {
>             return CLI_SHOWUSAGE;
>         }
>
> 
> Tilghman Lesher wrote:
>     I think it would still be better to make the change, along with the caller.  The only reason it was done that way originally was as an optimization step (the string never changes, so don't reformat the string multiple times: in each callback).

Eh, I don't really care much either way. I went ahead and made the change. :-)


- Terry


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1269/#review3719
-----------------------------------------------------------


On 2011-06-14 11:26:02, Terry Wilson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1269/
> -----------------------------------------------------------
> 
> (Updated 2011-06-14 11:26:02)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> There have been a couple of bugs reported lately against the Berkeley DB version that Asterisk uses for its internal database. These bugs have been tracked to the actual Berkeley code Asterisk uses, which is very very old and unmaintained. This patch replaces the Berkeley code with SQLite3 while maintaining exactly the same API. If there is no sqlite3 version of the database already and there is a Berkeley DB version, it automatically converts the old database to the new format.
> 
> If this goes in, I would then like to update Olle's patch for Realtime support for astdb as well and get that in.
> 
> 
> Diffs
> -----
> 
>   /configure UNKNOWN 
>   /trunk/configure.ac 323369 
>   /trunk/main/Makefile 323369 
>   /trunk/main/db.c 323369 
>   /trunk/tests/test_db.c 323369 
> 
> Diff: https://reviewboard.asterisk.org/r/1269/diff
> 
> 
> Testing
> -------
> 
> All astdb tests pass and I have tested converting several astdb databases.
> 
> 
> Thanks,
> 
> Terry
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20110614/2847aa9f/attachment-0001.htm>