On Thu, 12 Oct 2006, Kamesh Jayachandran wrote:
> Daniel Rall wrote:
> >On Wed, 11 Oct 2006, Kamesh Jayachandran wrote:
> >
> >>Daniel Rall wrote:
...
> >>>Is it 1) appropriate and 2) necessary to push knowledge of skels
> >>>corresponding to specific properties (e.g. SVN_FS_PROP_TXN_MERGEINFO)
> >>>down into fs_skels.c (rather than knowledge which ends at the concept
> >>>of "proplists", as on trunk)? To me, that file seems like a more
> >>>generic utility which shouldn't contain such specific knowledge.
> >>>
> >>I think somehow we can't avoid that. Someway or the other parsing
> >>code(which should sit inside the util/fs_skel.c) should know about
> >>'SVN_FS_PROP_TXN_MERGEINFO'. Or we should treat the value of
> >>'svn:txn-mergeinfo' as 'atom' and parse it outside?
> >>
> >
> >I chatted with Mike Pilato about this a bit. If we're initially using
> >sqlite for the merge info index, we should not be attempting to
> >store/retrieve merge info properties as structured data! They should
> >be stored simply as atoms.
> >
> >If we wanted to jump directly to storing merge info in BerkeleyDB,
> >we'd need to replace all usage of the sqlite-based svn_fs_merge_info.h
> >APIs in the BDB backend. While we continue prototyping an end-to-end
> >Merge Tracking solution, I would prefer to avoid this redundancy for
> >the sake of simplicity.
>
> Fine, I can implement 'svn:txn-mergeinfo' as like every other property
> and put a temporary parsing function to understand this 'atom' and give
> it as 'apr_hash_t'.
The apr_hash_t serialization you used looks great.
> >Along those lines, I question the safety of changing the signature of
> >svn_fs_base__dag_commit_txn() to remove its pool argument. This
> >change makes the assumption that anything allocated from TRAIL->pool
> >has the same lifetime as TRAIL, which may not always be correct.
>
> Just out of curiosity would like to know when trail->pool would have a
> short life time than trail itself so that come call back routines can
> cause memory corruption? Is ok till we have BDB backed mergeinfo?
trail->pool shouldn't typically be destroyed before we're done using
trail.
> >While it is clearly okay for the current caller, txn_body_commit()
> >(which already passes in trail->pool), I hesitate to make this type of
> >interface change for a "protected" API (it could later cause bugs).
> >If the function was file-static (completely private), I would be in
> >favor of this.
> >
> >Also, the interface into svn_fs_base__dag_*() seems to consistently
> >use txn_id, rather than txn. You seem to have made a change to txn to
> >accomodate svn_fs_merge_info__update_index(). Is this interface
> >discrepancy really worth it?
>
> Can I get svn_fs_txn_t* from txn_id?. I tried but got SIGABORT as we had
> already opened a trail.
Okay. I didn't explore why, but left it as you changed it (for the
time being).
- application/pgp-signature attachment: stored
Received on Fri Oct 20 01:23:46 2006