_check_ini_cache Regression

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

_check_ini_cache Regression

Joshua Colp
Kia ora,

First time posting here so sorry if this is the incorrect place (since
I'm discussing a regression and specific code I thought your dev list
was most appropriate).

In the 2.3.3 release a change was made to the _check_ini_cache function
in odbcinst/SQLGetPrivateProfileString.c to fix an issue.

The code before the change was:

         if ( nRetBuffer != ini_cache -> buffer_size )
             continue;

This was changed to:

         if ( nRetBuffer <= ini_cache -> buffer_size )
             continue;

Unfortunately this has caused a regression where cached entries are not
actually being used, causing the linked list to grow. In my case
nRetBuffer is 1024 and ini_cache->buffer_size is 1024 as expected. This
results in them being evaluated as equal and therefore skipped. Since
I'm testing an environment where many disconnect/connect attempts are
occurring at the same time this spirals and the cache grows out of control.

Looking at the code I think the following is the right fix for it but
I'm still looking to confirm:

         if ( nRetBuffer < ini_cache -> buffer_size )
             continue;

Eyes and feedback on this would be appreciated.

Cheers and have a great day,

--
Joshua Colp
Digium, Inc. | Senior Software Developer
445 Jan Davis Drive NW - Huntsville, AL 35806 - US
Check us out at: www.digium.com & www.asterisk.org
_______________________________________________
unixODBC-dev mailing list
[hidden email]
http://mailman.unixodbc.org/mailman/listinfo/unixodbc-dev
Reply | Threaded
Open this post in threaded view
|

Re: _check_ini_cache Regression

Nick Gorham-2
On 02/06/16 16:53, Joshua Colp wrote:

> Kia ora,
>
> First time posting here so sorry if this is the incorrect place (since
> I'm discussing a regression and specific code I thought your dev list
> was most appropriate).
>
> In the 2.3.3 release a change was made to the _check_ini_cache
> function in odbcinst/SQLGetPrivateProfileString.c to fix an issue.
>
> The code before the change was:
>
>         if ( nRetBuffer != ini_cache -> buffer_size )
>             continue;
>
> This was changed to:
>
>         if ( nRetBuffer <= ini_cache -> buffer_size )
>             continue;
>
> Unfortunately this has caused a regression where cached entries are
> not actually being used, causing the linked list to grow. In my case
> nRetBuffer is 1024 and ini_cache->buffer_size is 1024 as expected.
> This results in them being evaluated as equal and therefore skipped.
> Since I'm testing an environment where many disconnect/connect
> attempts are occurring at the same time this spirals and the cache
> grows out of control.
>
> Looking at the code I think the following is the right fix for it but
> I'm still looking to confirm:
>
>         if ( nRetBuffer < ini_cache -> buffer_size )
>             continue;

Yes, I would agree with you. TBH, the code is a bit odd there anyway, as
the size of the buffer is less important than the size of the string
that will be placed into it. But I think your fix should do the job. Let
me know how you get on and I will make the change permanent it it works
as expected.

--
Nick
_______________________________________________
unixODBC-dev mailing list
[hidden email]
http://mailman.unixodbc.org/mailman/listinfo/unixodbc-dev
Reply | Threaded
Open this post in threaded view
|

Re: _check_ini_cache Regression

Joshua Colp
Nick Gorham wrote:

<snip>

>> if ( nRetBuffer < ini_cache -> buffer_size )
>> continue;
>
> Yes, I would agree with you. TBH, the code is a bit odd there anyway, as
> the size of the buffer is less important than the size of the string
> that will be placed into it. But I think your fix should do the job. Let
> me know how you get on and I will make the change permanent it it works
> as expected.

Thanks for the quick response! My testing so far has shown it to resolve
the issue and performance has returned to what it was before the change.

--
Joshua Colp
Digium, Inc. | Senior Software Developer
445 Jan Davis Drive NW - Huntsville, AL 35806 - US
Check us out at: www.digium.com & www.asterisk.org
_______________________________________________
unixODBC-dev mailing list
[hidden email]
http://mailman.unixodbc.org/mailman/listinfo/unixodbc-dev