Joe Swatosh wrote:
> Hi
>
> On Wed, Oct 29, 2008 at 9:54 PM, Joe Swatosh <joe.swatosh_at_gmail.com> wrote:
>> Howdy Hyrum, kou,
>>
>> On Mon, Oct 27, 2008 at 8:08 AM, Hyrum K. Wright
>> <hyrum_wright_at_mail.utexas.edu> wrote:
>>> Joe Swatosh wrote:
>>>> Hi Hyrum,
>>>>
>>>> On Sat, Oct 18, 2008 at 6:12 AM, <hwright_at_tigris.org> wrote:
>>>>> Author: hwright
>>>>> Date: Sat Oct 18 06:11:59 2008
>>>>> New Revision: 33730
>>>>>
>>>>> Log:
>>>>> Merge the fs-rep-sharing branch to trunk.
>>>>>
>>>>> This gives us significant space savings on both bdb and fsfs. See the
>>>>> branch for individual log messages.
>>>>>
>>>> Obviously, this commit broke the windows build, but when it was fixed
>>>> in r33743 none of the Ruby bindings tests would even run anymore. I
>>>> instrumented the calls to sqlite3_open_v2() and sqlite3_close() and
>>>> many of the calls from the bindings into svn. Below is the output
>>>> along with some comments by me marked by #'s. I hope I'm talking
>>>> sense. This was run against trunk at r33894.
>>>>
>>>>
>
> snip
>
>>>> I'm just a bystander watching the development of svn proper, but I wonder
>>>> if there is a better pool to put the clean up of the cache opened by the
>>>> commit call above? It seems like ideally, it should be released before
>>>> the invocation of commit completes (like checkout).
>>> The rep-sharing sqlite database currently closes itself when the pool provided
>>> to svn_fs_new() or svn_fs_open() is destroyed. I suppose that if a caller
>>> doesn't destroy the pool, but instead uses it on a subsequent call to
>>> svn_fs_open(), then we'll have two open handles to the sqlite database. That in
>>> and of itself shouldn't be too much of a problem, but I'm wondering if the
>>> cleanup function we register with APR is getting confused.
>>>
>>> Thoughts?
>>>
>> Yup, the sqlite database is closed, but for whatever reason
>> svn_client_commit4 and svn_client_list2 (and others) are just passing
>> along the pool that was passed to them. I don't think the cleanup
>> function is confused, I just think the lifetime of the pool we are
>> passing in is too long. This might not even be problem on non-Windows
>> OSes that can delete open files.
>>
>> The apparent reason that svn_client_checkout3 was working for me was
>> that it calls svn_client__checkout_internal, which creates a temporary
>> "session_pool" that is passed to svn_ra_open3 and so the sqlite
>> database is closed before returning to Ruby.
This part is odd, because the SQLite database should be created in the same pool
that the filesystem is, and should be closed only when the filesystem is closed.
If there's a difference in there somewhere, that's a bug.
>> I started down the path of creating a patch for the svn_client library
>> mimicking that pattern of creating session_pools, but when I got to
>> svn_client_open_ra_session, I started doubting the approach. It seems
>> to me that if we are returning an RA session object that perhaps the
>
> Trying to make this a little more concrete, I've attached the patch in
> progress. The more I think about it the less I like it. It seems
> like there may be synchronization issues with having different
> functions accessing the cache with different file handles.
We've got some validation to do, to be sure (and I believe that's part of the
notes that Dave made about FSFS in TODO-1.6). However, SQLite handles the
multiple access issues for us. We're not working with different file handles,
we're working with different database handles, and the SQLite library ensures
that we can do that.
Perchance, are experiencing this problem on any kind of network file system?
-Hyrum
Received on 2008-11-03 14:46:54 CET