I have prepared a patch to show BDB verbose errors. It needs discussion, as
there is a notable problem, described below.
First, some sample output:
ampere:~/broken_repos maxb$ svnadmin verify svnrepos
svn: Berkeley DB error while checkpointing after Berkeley DB transaction for
filesystem svnrepos/db:
Invalid argument
svn: DB_ENV->log_flush: LSN of 77/697329 past current end-of-log of
77/608462
svn: Database environment corrupt; the wrong log files may have been removed
or incompatible database files imported from another environment
svn: representations: unable to flush page: 0
svn: txn_checkpoint: failed to flush the buffer cache Invalid argument
The errors from "svn: DB_ENV" to the end originate from BDB.
Log message:
[[[
Gather extended Berkeley DB error information using BDB's error callback
mechanism. Wrap the gathered info into the standard svn_error_t return.
* subversion/libsvn_fs_base/bdb/bdb-err.c (svn_fs_bdb__dberrf):
Add a svn_fs_t argument. Use it to retrieve pending errors stored from
the
bdb errcall, and wrap them.
(svn_fs_bdb__wrap_db): Pass svn_fs_t to svn_fs_bdb__dberrf.
* subversion/libsvn_fs_base/bdb/bdb-err.h (svn_fs_bdb__dberrf):
Change prototype as above.
* subversion/libsvn_fs_base/fs.c (base_bdb_set_errcall):
Set our user_errcall_handler, not BDB's internal handler, which we now
use
for our own purposes.
(bdb_error_gatherer): New function, called by BDB with extended error
info.
(allocate_env): Register our baton and callback function with BDB.
* subversion/libsvn_fs_base/fs.h (base_fs_data_t): Add new data fields.
"bdb_errcall_pending_errors" stores errors gathered in the callback
until
the Berkeley DB function returns (with an error code), and the process
of
wrapping the error collects the stored errors.
"user_errcall_handler" stores a callback registered with our public
svn_fs_set_berkeley_errcall API, now that we are using BDB's internal
error
callback for ourself.
]]]
Now, a note: This only takes effect when we open a svn_fs_t. For some small
operations, we open a BDB environment without wrapping it in a svn_fs_t:
base_bdb_recover
bdb_catastrophic_recover
base_bdb_logfiles
check_env_flags
get_db_pagesize
base_delete_fs
Fortunately, since we only ever do this within the lifetime of a single
function, writing the extra code for this will be uncomplicated. I'll get to
work on this, but it doesn't hinder discussion on the main part of patch
meanwhile.
And, now, the big problem:
We access svn data structures from within the callback, by using a pointer
passed via BDB's errpfx setting. This is a char* variable, though, *not* a
void*, and in some circumstances, is printf-ed by BDB. A quick look at the
bdb source seems to show that this will never occur if an errcall has been
set, so we might be OK. But, that's only based on a very quick
grep-and-look, and I've ignored the Perl/Java stuff in the bdb tarball when
doing so. Before we adopt this API misuse for real, we should seek
clarification from SleepyCat.
There is a safe, but ugly, workaround:
Pseudo-C:
struct {
char[6] pfx = "svn: \0";
base_fs_data_t *baton;
} svn_bdb_errpfx_t;
Now, we pass that to set_errpfx, and any attempt to treat it as a string
will work, but our callback function can know to look beyond the end of the
string for the pointer it needs.
Opinions?
1. Get SleepyCat to legitimize using a void* errpfx with BDB's errcall
feature?
2. Adopt the kludge outlined above?
3. Something else?
And the patch itself:
[[[
Index: subversion/libsvn_fs_base/bdb/bdb-err.c
===================================================================
--- subversion/libsvn_fs_base/bdb/bdb-err.c (revision 10521)
+++ subversion/libsvn_fs_base/bdb/bdb-err.c (working copy)
@@ -55,12 +55,18 @@
svn_error_t *
-svn_fs_bdb__dberrf (int db_err, const char *fmt, ...)
+svn_fs_bdb__dberrf (svn_fs_t *fs, int db_err, const char *fmt, ...)
{
va_list ap;
char *msg;
- svn_error_t *err = svn_error_create (bdb_err_to_apr_err (db_err), 0,
NULL);
+ base_fs_data_t *bfd = fs->fsap_data;
+ svn_error_t *child_errors;
+ child_errors = bfd->bdb_errcall_pending_errors;
+ bfd->bdb_errcall_pending_errors = NULL;
+ svn_error_t *err = svn_error_create (bdb_err_to_apr_err (db_err),
+ child_errors, NULL);
+
va_start (ap, fmt);
msg = apr_pvsprintf (err->pool, fmt, ap);
va_end (ap);
@@ -74,7 +80,8 @@
{
if (! db_err)
return SVN_NO_ERROR;
- return svn_fs_bdb__dberrf (db_err,
+
+ return svn_fs_bdb__dberrf (fs, db_err,
"Berkeley DB error while %s for filesystem
%s:\n",
operation, fs->path ? fs->path : "(none)");
}
Index: subversion/libsvn_fs_base/bdb/bdb-err.h
===================================================================
--- subversion/libsvn_fs_base/bdb/bdb-err.h (revision 10521)
+++ subversion/libsvn_fs_base/bdb/bdb-err.h (working copy)
@@ -50,7 +50,7 @@
There is no separator between the two messages; if you want one,
you should include it in FMT. */
-svn_error_t *svn_fs_bdb__dberrf (int db_err,
+svn_error_t *svn_fs_bdb__dberrf (svn_fs_t *fs, int db_err,
const char *fmt, ...);
Index: subversion/libsvn_fs_base/fs.c
===================================================================
--- subversion/libsvn_fs_base/fs.c (revision 10521)
+++ subversion/libsvn_fs_base/fs.c (working copy)
@@ -288,7 +288,7 @@
base_fs_data_t *bfd = fs->fsap_data;
SVN_ERR (svn_fs_base__check_fs (fs));
- bfd->env->set_errcall(bfd->env, db_errcall_fcn);
+ bfd->user_errcall_handler = db_errcall_fcn;
return SVN_NO_ERROR;
}
@@ -310,6 +310,24 @@
return db_err;
}
+
+static void
+bdb_error_gatherer (const char *pfx_baton, char *msg)
+{
+ base_fs_data_t *bfd = (base_fs_data_t*) pfx_baton;
+
+ svn_error_t *new_err = svn_error_create (SVN_NO_ERROR, NULL, msg);
+
+ if (bfd->bdb_errcall_pending_errors)
+ svn_error_compose(bfd->bdb_errcall_pending_errors, new_err);
+ else
+ bfd->bdb_errcall_pending_errors = new_err;
+
+ if (bfd->user_errcall_handler)
+ bfd->user_errcall_handler(0, msg);
+}
+
+
/* Allocate a Berkeley DB environment object for the filesystem FS,
and set up its default parameters appropriately. */
static svn_error_t *
@@ -321,6 +339,9 @@
SVN_ERR (BDB_WRAP (fs, "allocating environment object",
create_env (&bfd->env)));
+ bfd->env->set_errpfx (bfd->env, (char*)bfd);
+ bfd->env->set_errcall (bfd->env, bdb_error_gatherer);
+
/* If we detect a deadlock, select a transaction to abort at random
from those participating in the deadlock. */
SVN_ERR (BDB_WRAP (fs, "setting deadlock detection policy",
Index: subversion/libsvn_fs_base/fs.h
===================================================================
--- subversion/libsvn_fs_base/fs.h (revision 10521)
+++ subversion/libsvn_fs_base/fs.h (working copy)
@@ -55,6 +55,17 @@
/* The filesystem UUID (or NULL if not-yet-known; see svn_fs_get_uuid).
*/
const char *uuid;
+
+ /* Berkeley DB returns extended error info by callback before returning
+ an error code from the failing function. We hold the extended info
+ here until the Berkeley DB function returns, and we can return the
+ resulting svn_error_t. */
+ svn_error_t *bdb_errcall_pending_errors;
+
+ /* We permitted clients of our library to install a Berkeley errcall.
+ Since we now use the errcall ourselves, we must store and invoke a
user
+ errcall, to maintain our API guarantees. */
+ void (*user_errcall_handler) (const char *errpfx, char *msg);
} base_fs_data_t;
]]]
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Aug 7 20:55:34 2004