I realized having ESTALE in the names of these macros no longer
made sense, so I renamed them. I also expanded the comments. I
suspect ESTALE is defined on all platforms svn would run on, but
I'm not sure. So, use of the macros is still dependent on
whether ESTALE is defined; see comments. I'll commit this
tomorrow if no one objects.
[[[
* subversion/libsvn_fs_fs/fs_fs.c
(SVN_RECOVERABLE_RETRY_COUNT): Rename from SVN_ESTALE_RETRY_COUNT.
(SVN_RETRY_RECOVERABLE): Rename from SVN_RETRY_ESTALE; also retry
on NOENT and EIO; and expand comments.
(SVN_IGNORE_RECOVERABLE): Rename from SVN_IGNORE_ESTALE and also ignore EIO.
(read_current, svn_fs_fs__revision_proplist,
get_and_increment_txn_key_body): Pass new filehandle argument to
SVN_RETYRY_RECOVERABLE.
]]]
Index: subversion/libsvn_fs_fs/fs_fs.c
===================================================================
--- subversion/libsvn_fs_fs/fs_fs.c (revision 29393)
+++ subversion/libsvn_fs_fs/fs_fs.c (working copy)
@@ -1069,8 +1069,19 @@
}
-/* SVN_ERR-like macros for dealing with ESTALE
+/* SVN_ERR-like macros for dealing with recoverable errors on mutable files
*
+ * Revprops, current, and txn-current files are mutable; that is, they
+ * change as part of normal fsfs operation, in constrat to revs files, or
+ * the format file, which are written once at create (or upgrade) time.
+ * When more than one host writes to the same repository, we will
+ * sometimes see these recoverable errors when accesssing these files.
+ *
+ * These errors all relate to NFS, and thus we only use this retry code if
+ * ESTALE is defined.
+ *
+ ** ESTALE
+ *
* In NFS v3 and under, the server doesn't track opened files. If you
* unlink(2) or rename(2) a file held open by another process *on the
* same host*, that host's kernel typically renames the file to
@@ -1080,49 +1091,60 @@
* For obvious reasons, this does not work *across hosts*. No one
* knows about the opened file; not the server, and not the deleting
* client. So the file vanishes, and the reader gets stale NFS file
- * handle. We have this problem with revprops files, current, and
- * txn-current.
+ * handle.
*
- * Wrap opens and reads of such files with SVN_RETRY_ESTALE and closes
- * with SVN_IGNORE_ESTALE. Call these macros within a loop of
- * SVN_ESTALE_RETRY_COUNT iterations (though, realistically, the
- * second try will succeed). Make sure you put a break statement
- * after the close, at the end of your loop. Immediately after your
- * loop, return err if err.
+ ** EIO, ENOENT
*
- * You must initialize err to SVN_NO_ERROR, as these macros do not.
+ * Some client implementations (at least the 2.6.18.5 kernel that ships
+ * with Ubuntu Dapper) sometimes give spurious ENOENT (only on open) or
+ * even EIO errors when trying to read these files that have been renamed
+ * over on some other host.
+ *
+ ** Solution
+ *
+ * Wrap opens and reads of such files with SVN_RETRY_RECOVERABLE and
+ * closes with SVN_IGNORE_RECOVERABLE. Call these macros within a loop of
+ * SVN_RECOVERABLE_RETRY_COUNT iterations (though, realistically, the
+ * second try will succeed). Make sure you put a break statement after
+ * the close, at the end of your loop. Immediately after your loop,
+ * return err if err.
+ *
+ * You must initialize err to SVN_NO_ERROR and filehandle to NULL, as
+ * these macros do not.
*/
-#define SVN_ESTALE_RETRY_COUNT 10
+#define SVN_RECOVERABLE_RETRY_COUNT 10
#ifdef ESTALE
-#define SVN_RETRY_ESTALE(err, expr) \
- { \
- /* Clear err here (svn_error_clear can safely be passed
- * SVN_NO_ERROR) rather than after finding ESTALE so we can return
- * the ESTALE error on the last iteration of the loop. */ \
- svn_error_clear(err); \
- err = (expr); \
- if (err) \
- { \
- if (APR_TO_OS_ERROR(err->apr_err) == ESTALE) \
- continue; \
- return err; \
- } \
+#define SVN_RETRY_RECOVERABLE(err, filehandle, expr) \
+ { \
+ svn_error_clear(err); \
+ err = (expr); \
+ if (err) \
+ { \
+ apr_status_t _e = APR_TO_OS_ERROR(err->apr_err); \
+ if ((_e == ESTALE) || (_e == EIO) || (_e == ENOENT)) { \
+ if (NULL != filehandle) \
+ (void)apr_file_close(filehandle); \
+ continue; \
+ } \
+ return err; \
+ } \
}
-#define SVN_IGNORE_ESTALE(err, expr) \
- { \
- svn_error_clear(err); \
- err = (expr); \
- if (err) \
- { \
- if (APR_TO_OS_ERROR(err->apr_err) != ESTALE) \
- return err; \
- } \
+#define SVN_IGNORE_RECOVERABLE(err, expr) \
+ { \
+ svn_error_clear(err); \
+ err = (expr); \
+ if (err) \
+ { \
+ apr_status_t _e = APR_TO_OS_ERROR(err->apr_err); \
+ if ((_e != ESTALE) && (_e != EIO)) \
+ return err; \
+ } \
}
#else
-#define SVN_RETRY_ESTALE(err, expr) SVN_ERR(expr)
-#define SVN_IGNORE_ESTALE(err, expr) SVN_ERR(expr)
+#define SVN_RETRY_RECOVERABLE(err, filehandle, expr) SVN_ERR(expr)
+#define SVN_IGNORE_RECOVERABLE(err, expr) SVN_ERR(expr)
#endif
/* Long enough to hold: "<svn_revnum_t> <node id> <copy id>\0"
@@ -1139,7 +1161,7 @@
static svn_error_t *
read_current(const char *fname, char **buf, apr_pool_t *pool)
{
- apr_file_t *revision_file;
+ apr_file_t *revision_file = NULL;
apr_size_t len;
int i;
svn_error_t *err = SVN_NO_ERROR;
@@ -1147,18 +1169,20 @@
*buf = apr_palloc(pool, CURRENT_BUF_LEN);
iterpool = svn_pool_create(pool);
- for (i = 0; i < SVN_ESTALE_RETRY_COUNT; i++)
+ for (i = 0; i < SVN_RECOVERABLE_RETRY_COUNT; i++)
{
svn_pool_clear(iterpool);
- SVN_RETRY_ESTALE(err, svn_io_file_open(&revision_file, fname,
+ SVN_RETRY_RECOVERABLE(err, revision_file,
+ svn_io_file_open(&revision_file, fname,
APR_READ | APR_BUFFERED,
APR_OS_DEFAULT, iterpool));
len = CURRENT_BUF_LEN;
- SVN_RETRY_ESTALE(err, svn_io_read_length_line(revision_file,
+ SVN_RETRY_RECOVERABLE(err, revision_file,
+ svn_io_read_length_line(revision_file,
*buf, &len, iterpool));
- SVN_IGNORE_ESTALE(err, svn_io_file_close(revision_file, iterpool));
+ SVN_IGNORE_RECOVERABLE(err, svn_io_file_close(revision_file, iterpool));
break;
}
@@ -2148,7 +2172,7 @@
svn_revnum_t rev,
apr_pool_t *pool)
{
- apr_file_t *revprop_file;
+ apr_file_t *revprop_file = NULL;
apr_hash_t *proplist;
svn_error_t *err = SVN_NO_ERROR;
int i;
@@ -2158,13 +2182,12 @@
proplist = apr_hash_make(pool);
iterpool = svn_pool_create(pool);
- for (i = 0; i < SVN_ESTALE_RETRY_COUNT; i++)
+ for (i = 0; i < SVN_RECOVERABLE_RETRY_COUNT; i++)
{
svn_pool_clear(iterpool);
- /* Clear err here (svn_error_clear can safely be passed
- * SVN_NO_ERROR) rather than after finding ESTALE so we can
- * return the ESTALE error on the last iteration of the loop. */
+ /* Clear err here rather than after finding a recoverable error so
+ * we can return that error on the last iteration of the loop. */
svn_error_clear(err);
err = svn_io_file_open(&revprop_file, path_revprops(fs, rev, iterpool),
APR_READ | APR_BUFFERED, APR_OS_DEFAULT,
@@ -2178,20 +2201,22 @@
_("No such revision %ld"), rev);
}
#ifdef ESTALE
- else if (APR_TO_OS_ERROR(err->apr_err) == ESTALE)
+ else if (APR_TO_OS_ERROR(err->apr_err) == ESTALE
+ || APR_TO_OS_ERROR(err->apr_err) == EIO
+ || APR_TO_OS_ERROR(err->apr_err) == ENOENT)
continue;
#endif
return err;
}
SVN_ERR(svn_hash__clear(proplist));
- SVN_RETRY_ESTALE(err,
- svn_hash_read2(proplist,
- svn_stream_from_aprfile(revprop_file,
- iterpool),
- SVN_HASH_TERMINATOR, pool));
+ SVN_RETRY_RECOVERABLE(err, revprop_file,
+ svn_hash_read2(proplist,
+ svn_stream_from_aprfile(revprop_file,
+ iterpool),
+ SVN_HASH_TERMINATOR, pool));
- SVN_IGNORE_ESTALE(err, svn_io_file_close(revprop_file, iterpool));
+ SVN_IGNORE_RECOVERABLE(err, svn_io_file_close(revprop_file, iterpool));
break;
}
@@ -3519,7 +3544,7 @@
{
struct get_and_increment_txn_key_baton *cb = baton;
const char *txn_current_filename = path_txn_current(cb->fs, pool);
- apr_file_t *txn_current_file;
+ apr_file_t *txn_current_file = NULL;
const char *tmp_filename;
char next_txn_id[MAX_KEY_SIZE+3];
svn_error_t *err = SVN_NO_ERROR;
@@ -3530,20 +3555,23 @@
cb->txn_id = apr_palloc(cb->pool, MAX_KEY_SIZE);
iterpool = svn_pool_create(pool);
- for (i = 0; i < SVN_ESTALE_RETRY_COUNT; ++i)
+ for (i = 0; i < SVN_RECOVERABLE_RETRY_COUNT; ++i)
{
svn_pool_clear(iterpool);
- SVN_RETRY_ESTALE(err, svn_io_file_open(&txn_current_file,
+ SVN_RETRY_RECOVERABLE(err, txn_current_file,
+ svn_io_file_open(&txn_current_file,
txn_current_filename,
APR_READ | APR_BUFFERED,
APR_OS_DEFAULT, iterpool));
len = MAX_KEY_SIZE;
- SVN_RETRY_ESTALE(err, svn_io_read_length_line(txn_current_file,
+ SVN_RETRY_RECOVERABLE(err, txn_current_file,
+ svn_io_read_length_line(txn_current_file,
cb->txn_id,
&len,
iterpool));
- SVN_IGNORE_ESTALE(err, svn_io_file_close(txn_current_file, iterpool));
+ SVN_IGNORE_RECOVERABLE(err, svn_io_file_close(txn_current_file,
+ iterpool));
break;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-02-19 19:37:43 CET