FS Re-factoring
Below is a simplified view of the Fs and those layers directly affected by it.
As noted, BDB dependencies are located at various levels in the system. To ease implementation of new FS back ends, I feel more BDB isolation is required.
Here are the basic changes I believe need to occur. (I have begun making the changes locally)
- All explicit references, to BDB structures in svn_fs_t , need to be removed. And replaced with a void reference to “FS type specific data”.
- All functions containing Berkeley should be re-named. Functions that have no usefulness for other FS types should make these no-ops
- Provide mechanism for Fs implementers to supplant implementations of most of the exposed functions. Some non-exposed functions will most likely evolve as well. Functions on Ids and such won’t have a reference to a svn_fs_t structure embedded in any of the calling parameters. So they will not be able to be replaced. None of them should be incompatible with other Fs implementations.
- Move all BDB related code to BDB sub-dir.
- Attempt to extract or leave in place that code which can or will prove useful to other FS implementers. For this phase of re-factoring, avoid extracting code where “significant” logic has to change to remove BDB semantics. Instead, where possible, place modified versions of the code in a “common” sub-dir for use in new Fs implementations and later use in the BDB implementation. This should minimize new bugs caused by the re-factoring effort.
- Add svn_config_t parameter to the create calls. This will allow configuration parameters specific to the FS implementation to be passed in. I will Explain this in a soon to come document on a SQL FS.
- long-term the build system needs to change the server build requirements.
The end result should be more like this:
Since I think c-mike and ghudson seem to somewhat agree with where I’m headed.
Here is my response to gsteins concerns:
>> I'm looking at creating a vtable for most of the exposed fs functions
>> exposed in svn_fs.h. It would be initialized by the create and/or open
>> process.
>Gah! No way. The FS interface is the wrong level of abstraction. There is a
l>ot of code in FS to handle the DAG concept, to do merging, and other
>behaviors. If you switch at the FS level, then every backend will need to
>implement that stuff.
I disagree. Allowing “FS implementation” specific versions of functions does not eliminate code reuse.
>I would suggest that you start with Bill Tutt's model and go from there.
>That diagram specifies the underlying concepts that we're operating with.
>From there, we apply that model on top of BDB. Or a relational backend or
>whatever.
>The problem, of course, is that our current code organization and internal
>concepts do not mirror the model well. And that is probably where
>refactoring would begin.
There is a well defined API, are you telling me it isn’t correct?
I see Bill’s stuff as an implementation of the API not the API itself.
As Greg Hudson stated :
“I think there's some stuff which should move out of the FS, but probably
less than you are talking about. The FS interface is a nice
concept-oriented interface which gives you a lot of implementation
flexibility; Bill Tutt's model (as I understand it) is much more
restrictive in terms of implementation choices.”
>> I want to add an optional svn_config_t parameter on the repos_create
>> call. If it's NULL everything would be as it exists today. Minus the
>> added indirection of course.
>Not sure. Would need to see this one.
I believe this to be absolutely necessary for my plans and will not hurt the existing BDB stuff at all. I will explain more in the SQL document to come.
>> I want to locate the vtable in the svn_fs_t structure. I'm struggling a
>> little with what to do with DB specific references in svn_fs_t. I'm
>> leaning towards (void*) baton cast at the "knowing" level.
>Have a complete structure which contains the vtable, and a companion pointer
>for provider-specific data. That allows type-checking on the vtable stuff,
>but retains the provider-private concept.
>Any data outside of the vtable that is needed in "the new scheme" would just
>go into svn_fs_t rather than a new sub-structure.
>>...
>> I figure the exposed functions and any bdb related support functions
>> would be renamed and moved to the bdb directory.
>Most are there already.
That depends on how you define “Most”J
Here are the changes I have made to the FS structure so far:
struct svn_fs_t
{
/* A pool managing this filesystem. Freeing this pool must
completely clean up the filesystem, including any database
or system resources it holds. */
apr_pool_t *pool;
/* The path to the repository's top-level directory. */
char *path;
/* FS type specific structures */
int fs_type;
/* Will be cast by db-specific fs routines */
void *db_cntxt;
svn_fs__api_vtab_t *vtab;
/* A callback function for printing warning messages, and a baton to
pass through to it. */
svn_fs_warning_callback_t warning;
void *warning_baton;
/* A kludge for handling errors noticed by APR pool cleanup functions.
The APR pool cleanup functions can only return an apr_status_t
value, not a full svn_error_t value. This makes it difficult to
propagate errors detected by fs_cleanup to someone who can handle
them.
If FS->cleanup_error is non-zero, it points to a location where
fs_cleanup should store a pointer to an svn_error_t object, if it
generates one. Normally, it's zero, but if the cleanup is
invoked by code prepared to deal with an svn_error_t object in
some helpful way, it can create its own svn_error_t *, set it to
zero, set cleanup_error to point to it, free the pool (thus
invoking the cleanup), and then check its svn_error_t to see if
anything went wrong.
Of course, if multiple errors occur, this will only report one of
them, but it's better than nothing. In the case of a cascade,
the first error message is probably the most helpful, so
fs_cleanup won't overwrite a pointer to an existing svn_error_t
if it finds one. */
svn_error_t **cleanup_error;
};
For the BDB FS db_cntxt would point to:
typedef struct
{
/* A Berkeley DB environment for all the filesystem's databases.
This establishes the scope of the filesystem's transactions. */
DB_ENV *env;
/* The filesystem's various tables. See `structure' for details. */
DB *changes;
DB *copies;
DB *nodes;
DB *representations;
DB *revisions;
DB *strings;
DB *transactions;
} svn_bdb_cntxt_t;
I have begun the process of moving code to the bdb directory and building a vatable init function for the BDB FS. It mirrors the API for the most part.
The vtable looks like this so far:
struct svn_fs__api_vtab_t
{
/* non api functions */
/* may be no-op */
apr_status_t (*cleanup_apr) (void *data);
svn_error_t * (*cleanup_fs) (svn_fs_t *fs);
/* exposed functions */
svn_fs_t *(*new) (apr_pool_t *pool);
svn_error_t *(*close_fs) (svn_fs_t *);
void (*set_warning_func) (svn_fs_t *fs,svn_fs_warning_callback_t warning,
void *warning_baton);
/* begin functions that had berkeley in their name */
svn_error_t *(*create_fs) (svn_fs_t *fs, const char *path, svn_config_t *cfg);
svn_error_t *(*open_fs) (svn_fs_t *fs, const char *path);
const char *(*fs_path) (svn_fs_t *fs, apr_pool_t *pool);
/* this guy is berkeley oriented. I need to generisize this thing */
/* Flag */
svn_error_t *(*set_errcall) (svn_fs_t *fs,
void (*handler) (const char *errpfx,
char *msg));
svn_error_t *(*delete) (const char *PATH, apr_pool_t *pool);
/* Flag */
svn_error_t *(*recover) (const char *path, apr_pool_t *pool);
/* end functions that had berkeley in their name */
/* Nodes. */
int (*compare_ids) (const svn_fs_id_t *a, const svn_fs_id_t *b);
int (*check_related) (const svn_fs_id_t *id1, const svn_fs_id_t *id2);
svn_fs_id_t *(*parse_id) (const char *data, apr_size_t len,
apr_pool_t *pool);
svn_string_t *(*unparse_id) (const svn_fs_id_t *id, apr_pool_t *pool);
/* Transactions. */
svn_error_t *(*begin_txn) (svn_fs_txn_t **txn_p, svn_fs_t *fs,
svn_revnum_t rev, apr_pool_t *pool);
svn_error_t *(*commit_txn) (const char **conflict_p,
svn_revnum_t *new_rev, svn_fs_txn_t *txn);
svn_error_t *(*abort_txn) (svn_fs_txn_t *txn);
svn_error_t *(*txn_name) (const char **name_p, svn_fs_txn_t *txn,
apr_pool_t *pool);
svn_fs_t *(*txn_fs) (svn_fs_txn_t *txn);
apr_pool_t *(*txn_pool) (svn_fs_txn_t *txn);
svn_revnum_t (*txn_base_revision) (svn_fs_txn_t *txn);
svn_error_t *(*open_txn) (svn_fs_txn_t **txn, svn_fs_t *fs,
const char *name, apr_pool_t *pool);
svn_error_t *(*close_txn) (svn_fs_txn_t *txn);
svn_error_t *(*list_transactions) (apr_array_header_t **names_p,
svn_fs_t *fs, apr_pool_t *pool);
/* Transaction properties */
svn_error_t *(*txn_prop) (svn_string_t **value_p, svn_fs_txn_t *txn,
const char *propname, apr_pool_t *pool);
svn_error_t *(*txn_proplist) (apr_hash_t **table_p, svn_fs_txn_t *txn,
apr_pool_t *pool);
svn_error_t *(*change_txn_prop) (svn_fs_txn_t *txn, const char *name,
const svn_string_t *value,
apr_pool_t *pool);
/* Roots. */
svn_error_t *(*revision_root) (svn_fs_root_t **root_p, svn_fs_t *fs,
svn_revnum_t rev, apr_pool_t *pool);
svn_error_t *(*txn_root) (svn_fs_root_t **root_p, svn_fs_txn_t *txn,
apr_pool_t *pool);
void (*close_root) (svn_fs_root_t *root);
svn_fs_t *(*root_fs) (svn_fs_root_t *root);
int (*is_txn_root) (svn_fs_root_t *root);
int (*is_revision_root) (svn_fs_root_t *root);
const char *(*txn_root_name) (svn_fs_root_t *root, apr_pool_t *pool);
svn_revnum_t *(*revision_root_revision) (svn_fs_root_t *root);
/* Determining what has changed under a ROOT. */
svn_error_t *(*paths_changed) (apr_hash_t **changed_paths_p,
svn_fs_root_t *root,
apr_pool_t *pool);
svn_node_kind_t (*check_path) (svn_fs_root_t *root, const char *path,
apr_pool_t *pool);
svn_error_t *(*revisions_changed) (apr_array_header_t **revs,
svn_fs_root_t *root,
const apr_array_header_t *paths,
int cross_copy_history,
apr_pool_t *pool);
svn_error_t *(*is_dir) (int *is_dir, svn_fs_root_t *root,
const char *path, apr_pool_t *pool);
svn_error_t *(*is_file) (int *is_file, svn_fs_root_t *root,
const char *path, apr_pool_t *pool);
svn_error_t *(*node_id) (const svn_fs_id_t **id_p, svn_fs_root_t *root,
const char *path, apr_pool_t *pool);
svn_error_t *(*node_created_rev) (svn_revnum_t *revision,
svn_fs_root_t *root, const char *path,
apr_pool_t *pool);
svn_error_t *(*node_prop) (svn_string_t **value_p, svn_fs_root_t *root,
const char *path, const char *propname,
apr_pool_t *pool);
svn_error_t *(*node_proplist) (apr_hash_t **table_p, svn_fs_root_t *root,
const char *path, apr_pool_t *pool);
svn_error_t *(*change_node_prop) (svn_fs_root_t *root, const char *path,
const char *name,
const svn_string_t *value,
apr_pool_t *pool);
svn_error_t *(*props_changed) (int *changed_p, svn_fs_root_t *root1,
const char *path1, svn_fs_root_t *root2,
const char *path2, apr_pool_t *pool);
svn_error_t *(*copied_from) (svn_revnum_t *rev_p, const char **path_p,
svn_fs_root_t *root, const char *path,
apr_pool_t *pool);
svn_error_t *(*merge) (const char **conflict_p,
svn_fs_root_t *source_root,
const char *source_path,
svn_fs_root_t *target_root,
const char *target_path,
svn_fs_root_t *ancestor_root,
const char *ancestor_path,
apr_pool_t *pool);
svn_error_t *(*is_different) (int *is_different, svn_fs_root_t *root1,
const char *path1, svn_fs_root_t *root2,
const char *path2, apr_pool_t *pool);
/* Deltification of Storage. */
svn_error_t *(*deltify) (svn_fs_root_t *root, const char *path,
int recursive, apr_pool_t *pool);
svn_error_t *(*undeltify) (svn_fs_root_t *root, const char *path,
int recursive, apr_pool_t *pool);
/* Directories. */
svn_error_t *(*dir_entries) (apr_hash_t **entries_p, svn_fs_root_t *root,
const char *path, apr_pool_t *pool);
svn_error_t *(*make_dir) (svn_fs_root_t *root, const char *path,
apr_pool_t *pool);
svn_error_t *(*delete_node) (svn_fs_root_t *root, const char *path,
apr_pool_t *pool);
svn_error_t *(*delete_tree) (svn_fs_root_t *root, const char *path,
apr_pool_t *pool);
svn_error_t *(*rename) (svn_fs_root_t *root, const char *from,
const char *to, apr_pool_t *pool);
svn_error_t *(*copy) (svn_fs_root_t *from_root, const char *from_path,
svn_fs_root_t *to_root, const char *to_path,
apr_pool_t *pool);
svn_error_t *(*revision_link) (svn_fs_root_t *from_root,
svn_fs_root_t *to_root,
const char *path,
apr_pool_t *pool);
/* Files. */
svn_error_t *(*file_length) (apr_off_t *length_p, svn_fs_root_t *root,
const char *path, apr_pool_t *pool);
svn_error_t *(*file_contents) (svn_stream_t **contents,
svn_fs_root_t *root, const char *path,
apr_pool_t *pool);
svn_error_t *(*make_file) (svn_fs_root_t *root, const char *path,
apr_pool_t *pool);
svn_error_t *(*apply_textdelta) (svn_txdelta_window_handler_t *contents_p,
void **contents_baton_p,
svn_fs_root_t *root, const char *path,
apr_pool_t *pool);
svn_error_t *(*apply_text) (svn_stream_t **contents_p, svn_fs_root_t *root,
const char *path, apr_pool_t *pool);
svn_error_t *(*contents_changed) (int *changed_p, svn_fs_root_t *root1,
const char *path1, svn_fs_root_t *root2,
const char *path2, apr_pool_t *pool);
/* Filesystem revisions. */
svn_error_t *(*youngest_rev) (svn_revnum_t *youngest_p, svn_fs_t *fs,
apr_pool_t *pool);
svn_error_t *(*revision_prop) (svn_string_t **value_p, svn_fs_t *fs,
svn_revnum_t rev, const char *propname,
apr_pool_t *pool);
svn_error_t *(*revision_proplist) (apr_hash_t **table_p, svn_fs_t *fs,
svn_revnum_t rev, apr_pool_t *pool);
svn_error_t *(*change_rev_prop) (svn_fs_t *fs, svn_revnum_t rev,
const char *name,
const svn_string_t *value,
apr_pool_t *pool);
/* Computing deltas. */
svn_error_t *(*get_file_delta_stream) (svn_txdelta_stream_t **stream_p,
svn_fs_root_t *source_root,
const char *source_path,
svn_fs_root_t *target_root,
const char *target_path,
apr_pool_t *pool);
};