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.
>>
>>
>> # A bunch of initialization
>> Svn::Ext::Core.svn_nls_init start
>> Svn::Ext::Core.svn_nls_init end
>> Svn::Ext::Core.svn_default_charset start
>> Svn::Ext::Core.svn_default_charset end
>> Svn::Ext::Core.svn_locale_charset start
>> Svn::Ext::Core.svn_locale_charset end
>> Svn::Ext::Fs.svn_fs_initialize start
>> Svn::Ext::Fs.svn_fs_initialize end
>> Svn::Ext::Wc.svn_wc_swig_init_asp_dot_net_hack start
>> Svn::Ext::Wc.svn_wc_swig_init_asp_dot_net_hack end
>> Svn::Ext::Ra.svn_ra_initialize start
>> Svn::Ext::Ra.svn_ra_initialize end Loaded suite test_info.rb
>> Started
>> test_info(SvnInfoTest):
>> #
>> # Setting up for the test creates a repo (We weren't closing it after the
>> # create, but we do now).
>> #
>> Svn::Ext::Repos.svn_repos_create start sqlite3_open_v2(2ec9780)
>> Svn::Ext::Repos.svn_repos_create end
>> x.close start sqlite3_close(2ec9780)
>> x.close end
>> #
>> # Still in setup we open the repository and keep a reference for later
>> #
>> Svn::Ext::Repos.svn_repos_open start sqlite3_open_v2(2ec9780)
>> Svn::Ext::Repos.svn_repos_open end
>> Svn::Ext::Repos.svn_repos_fs_wrapper start
>> Svn::Ext::Repos.svn_repos_fs_wrapper end
>> Svn::Ext::Repos.svn_repos_pre_revprop_change_hook start
>> Svn::Ext::Repos.svn_repos_pre_revprop_change_hook end
>> Svn::Ext::Core.svn_config_ensure start
>> Svn::Ext::Core.svn_config_ensure end
>> Svn::Ext::Client.svn_client_set_log_msg_func3 start
>> Svn::Ext::Client.svn_client_set_log_msg_func3 end
>> Svn::Ext::Client.svn_client_set_notify_func2 start
>> Svn::Ext::Client.svn_client_set_notify_func2 end
>> Svn::Ext::Client.svn_client_set_cancel_func start
>> Svn::Ext::Client.svn_client_set_cancel_func end
>> Svn::Ext::Client.svn_client_set_log_msg_func3 start
>> Svn::Ext::Client.svn_client_set_log_msg_func3 end
>> Svn::Ext::Core.svn_swig_rb_auth_get_username_prompt_provider start
>> Svn::Ext::Core.svn_swig_rb_auth_get_username_prompt_provider end
>> Svn::Ext::Core.svn_auth_set_parameter start
>> Svn::Ext::Core.svn_auth_set_parameter end
>> Svn::Ext::Core.svn_auth_set_parameter start
>> Svn::Ext::Core.svn_auth_set_parameter end
>> #
>> # It looks like doing the checkout opens and closes the cache twice. (Reusing
>> # the same address).
>> #
>> Svn::Ext::Client.svn_client_checkout3
>> start sqlite3_open_v2(2eca1b8) sqlite3_close(2eca1b8) sqlite3_open_v2(2eca1b8) sqlite3_close(2eca1b8)
>> Svn::Ext::Client.svn_client_checkout3 end
>> Svn::Ext::Repos.svn_repos_svnserve_conf start
>> Svn::Ext::Repos.svn_repos_svnserve_conf end
>> Svn::Ext::Repos.svn_repos_conf_dir start
>> Svn::Ext::Repos.svn_repos_conf_dir end
>> Svn::Ext::Client.svn_client_set_log_msg_func3 start
>> Svn::Ext::Client.svn_client_set_log_msg_func3 end
>> Svn::Ext::Client.svn_client_set_notify_func2 start
>> Svn::Ext::Client.svn_client_set_notify_func2 end
>> Svn::Ext::Client.svn_client_set_cancel_func start
>> Svn::Ext::Client.svn_client_set_cancel_func end
>> Svn::Ext::Client.svn_client_set_log_msg_func3 start
>> Svn::Ext::Client.svn_client_set_log_msg_func3 end
>> Svn::Ext::Core.svn_swig_rb_auth_get_username_prompt_provider start
>> Svn::Ext::Core.svn_swig_rb_auth_get_username_prompt_provider end
>> Svn::Ext::Core.svn_auth_set_parameter start
>> Svn::Ext::Core.svn_auth_set_parameter end
>> Svn::Ext::Core.svn_auth_set_parameter start
>> Svn::Ext::Core.svn_auth_set_parameter end
>> Svn::Ext::Client.svn_client_add3 start
>> Svn::Ext::Client.svn_client_add3 end
>> #
>> # commit is opening the cache, but not closing it. I think this is what is
>> # causing the test failures, more below...
>> #
>> Svn::Ext::Client.svn_client_commit4 start sqlite3_open_v2(2eca1b8)
>> Svn::Ext::Client.svn_client_commit4 end
>> #
>> # Okay, another place that the bindings opened a repo, but didn't close
>> # it that is now better in my wc
>> #
>> make_info start
>> Svn::Ext::Repos.svn_repos_open start sqlite3_open_v2(2f67750)
>> Svn::Ext::Repos.svn_repos_open end
>> Svn::Ext::Repos.svn_repos_fs_wrapper start
>> Svn::Ext::Repos.svn_repos_fs_wrapper end
>> Svn::Ext::Fs.svn_fs_revision_root start
>> Svn::Ext::Fs.svn_fs_revision_root end
>> Svn::Ext::Fs.svn_fs_revision_prop start
>> Svn::Ext::Fs.svn_fs_revision_prop end
>> Svn::Ext::Fs.svn_fs_revision_prop start
>> Svn::Ext::Fs.svn_fs_revision_prop end
>> Svn::Ext::Core.svn_parse_date start
>> Svn::Ext::Core.svn_parse_date end
>> Svn::Ext::Fs.svn_fs_revision_prop start
>> Svn::Ext::Fs.svn_fs_revision_prop end
>> Svn::Ext::Fs.svn_fs_revision_root start
>> Svn::Ext::Fs.svn_fs_revision_root end
>> Svn::Ext::Repos.svn_repos_dir_delta start
>> Svn::Ext::Repos.svn_repos_dir_delta end
>> Svn::Ext::Fs.svn_fs_revision_root start
>> Svn::Ext::Fs.svn_fs_revision_root end
>> Svn::Ext::Repos.svn_repos_dir_delta start
>> Svn::Ext::Fs.svn_fs_copied_from start
>> Svn::Ext::Fs.svn_fs_copied_from end
>> Svn::Ext::Repos.svn_repos_dir_delta end
>> Svn::Ext::Repos.svn_repos_fs_wrapper start
>> Svn::Ext::Repos.svn_repos_fs_wrapper end
>> Svn::Ext::Fs.svn_fs_revision_root start
>> Svn::Ext::Fs.svn_fs_revision_root end
>> Svn::Ext::Repos.svn_repos_node_editor start
>> Svn::Ext::Repos.svn_repos_node_editor end
>> Svn::Ext::Repos.svn_repos_replay2 start
>> Svn::Ext::Repos.svn_repos_replay2 end
>> Svn::Ext::Repos.svn_repos_node_from_baton start
>> Svn::Ext::Repos.svn_repos_node_from_baton end
>> Svn::Ext::Fs.svn_fs_revision_root start
>> Svn::Ext::Fs.svn_fs_revision_root end
>> Svn::Ext::Fs.svn_fs_node_prop start
>> Svn::Ext::Fs.svn_fs_node_prop end
>> Svn::Ext::Fs.svn_fs_revision_root_revision start
>> Svn::Ext::Fs.svn_fs_revision_root_revision end
>> Svn::Ext::Fs.svn_fs_revision_root_revision start
>> Svn::Ext::Fs.svn_fs_revision_root_revision end
>> Svn::Ext::Fs.svn_fs_root_fs_wrapper start
>> Svn::Ext::Fs.svn_fs_root_fs_wrapper end
>> Svn::Ext::Fs.svn_fs_revision_prop start
>> Svn::Ext::Fs.svn_fs_revision_prop end
>> Svn::Ext::Core.svn_parse_date start
>> Svn::Ext::Core.svn_parse_date end
>> Svn::Ext::Fs.svn_fs_root_fs_wrapper start
>> Svn::Ext::Fs.svn_fs_root_fs_wrapper end
>> Svn::Ext::Fs.svn_fs_revision_prop start
>> Svn::Ext::Fs.svn_fs_revision_prop end
>> Svn::Ext::Core.svn_parse_date start
>> Svn::Ext::Core.svn_parse_date end
>> Svn::Ext::Fs.svn_fs_file_contents start
>> Svn::Ext::Fs.svn_fs_file_contents end
>> Svn::Ext::Core.svn_stream_read start
>> Svn::Ext::Core.svn_stream_read end
>> Svn::Ext::Core.svn_stream_close start
>> Svn::Ext::Core.svn_stream_close end
>> Svn::Ext::Core.svn_diff_file_diff_2 start
>> Svn::Ext::Core.svn_diff_file_diff_2 end
>> Svn::Ext::Fs.svn_fs_file_contents start
>> Svn::Ext::Fs.svn_fs_file_contents end
>> Svn::Ext::Core.svn_stream_read start
>> Svn::Ext::Core.svn_stream_read end
>> Svn::Ext::Core.svn_stream_close start
>> #
>> # closing repo opened by make_info() implementation....
>> #
>> Svn::Ext::Core.svn_stream_close end sqlite3_close(2f67750)
>> make_info end
>> Svn::Ext::Core.svn_time_from_cstring start
>> Svn::Ext::Core.svn_time_from_cstring end
>> #
>> # test teardown, closing the repo opened in the setup
>> #
>> @repos.close start sqlite3_close(2ec9780)
>> @repos.close end
>> #
>> # ohoh, can't delete the repo because the cache is still open.
>> #
>> Svn::Ext::Repos.svn_repos_delete start E
>>
>> Finished in 13.965 seconds.
>>
>> 1) Error:
>> test_info(SvnInfoTest):
>> Svn::Error::SvnError:
>> D:\SVN\src-trunk\subversion\libsvn_subr\io.c:1714: 720032: Can't
>> remove file 'repos\db\rep-cache.db': The process cannot access the
>> file because it is being used by another process.
>> D:/SVN/src-trunk/subversion/bindings/swig/ruby/svn/util.rb:88:in
>> `svn_repos_delete'
>> D:/SVN/src-trunk/subversion/bindings/swig/ruby/svn/util.rb:88:in `delete'
>> D:/SVN/src-trunk/subversion/bindings/swig/ruby/test/util.rb:114:in
>> `teardown_repository'
>> D:/SVN/src-trunk/subversion/bindings/swig/ruby/test/util.rb:48:in
>> `teardown_basic'
>> D:/SVN/src-trunk/subversion/bindings/swig/ruby/test\test_info.rb:13:in
>> `teardown'
>>
>> 1 tests, 4 assertions, 0 failures, 1 errors
>> #
>> # Ah, process wind down or GC is going to release that last link to the
>> # sqlite cache.
>> #
>> sqlite3_close(2eca1b8)
>>
>> ********************************************************************************
>>
>> 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.
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
handle to the cache ought to stick around for the lifetime of the
session object. Or if that isn't correct maybe the bindings should
create a pool for the duration? I looked at this a little, but I'm
not sure how to accomplish that. Or maybe I should just continue
creating session pools and post the patch?
More questions than answers, I'm afraid.
--
Joe
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-10-30 05:54:30 CET