On 01/06/2010 07:32 PM, Philip Martin wrote:
Kamesh Jayachandran<kam...@collab.net> writes:
This patch is with respect to the original thread
http://mail-archives.apache.org/mod_mbox/subversion-dev/201001.mbox/browser
This one I suppose:
http://mail-archives.apache.org/mod_mbox/subversion-dev/201001.mbox/<4b41f1bd.8090...@collab.net>
Yes, thanks for correcting.
It includes:
"We can proxy this request to the Master but we *should not* do
that if it is for read operation."
Does something go wrong if the read operation goes to the master or is
it just an efficiency thing?
It can go wrong in the following scenario,
If Slave is out-dated still checkouts from it should work without any
confusion. If we always proxy 'PROPFIND' to Master we may get some new
revision numbers operated against the Slave and hence loads of confusion.
Once this patch gets committed I can commit the mod_dav_svn change to
handle the original commit via outdated proxy issue.
Have we seen this other patch yet?
Not yet, Find it in the attachment. This fix expects the new header
'SVN-ACTION', if it does not exist makes use of 'connection persistence'
to find the commit activity and proxy selectively.
This Patch revs the following public APIs,
'svn_client_uuid_from_url', 'svn_client_open_ra_session' and 'svn_ra_open3'.
For ra_neon and ra_serf layers it sets the http client header
SVN-ACTION with the concerned svn command name.
The "SVN-ACTION" name is already used by the operational logging code,
I don't know if this matters.
May be in the *future* when the new client is everywhere we can change
the operational logging code to make use of this header.
May be I can change this to something SVN-COMMAND if there are serious
objections.
With& Without this patch mergeinfo_test-8 fails both over ra_neon and
ra_serf.
If there are no objections I will commit this change in next 2 days.
This ties an RA session to a single operation name -- that's a new
restriction on RA sessions isn't it? It may be the way our clients
work at present but clients don't have to work like that, a single
session could be used for multiple operations. A client may not even
know what operations are going to be carried out until some time after
the session has been opened.
May be we can provide an RA API to change the command names prior to
using the pre-existing ra_session for new command.
With regards
Kamesh Jayachandran
Index: subversion/mod_dav_svn/mirror.c
===================================================================
--- subversion/mod_dav_svn/mirror.c (revision 896432)
+++ subversion/mod_dav_svn/mirror.c (working copy)
@@ -30,7 +30,54 @@
#include "dav_svn.h"
+#define USERDATA_KEY "mod_dav_svn_userdata_key"
+typedef struct
+{
+ int mk_activity_has_happened;
+} ctx_data;
+
+static apr_status_t
+initialize_ctx_data(ctx_data **data,
+ request_rec *r,
+ apr_pool_t *pool)
+{
+ ctx_data *ctx = apr_pcalloc(pool, sizeof(*ctx));
+
+ if (apr_pool_userdata_set(ctx, USERDATA_KEY, NULL, pool) != APR_SUCCESS)
+ {
+ ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
+ "mod_dav_svn: Error setting connection pool userdata");
+ return HTTP_INTERNAL_SERVER_ERROR;
+ }
+ ctx->mk_activity_has_happened = 0;
+ *data = ctx;
+ return APR_SUCCESS;
+}
+
+static int
+get_userdata(ctx_data **ctx,
+ request_rec *r)
+{
+ void *data;
+ apr_pool_t *pool = r->connection->pool;
+
+ if (apr_pool_userdata_get(&data, USERDATA_KEY, pool) != APR_SUCCESS)
+ {
+ ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
+ "mod_dav_svn: Error fetching connection userdata");
+ return HTTP_INTERNAL_SERVER_ERROR;
+ }
+
+ if (data)
+ *ctx = data;
+ else
+ initialize_ctx_data(ctx, r, pool);
+
+ return OK;
+}
+
+
/* Tweak the request record R, and add the necessary filters, so that
the request is ready to be proxied away. MASTER_URI is the URI
specified in the SVNMasterURI Apache configuration value.
@@ -57,6 +104,7 @@
int dav_svn__proxy_merge_fixup(request_rec *r)
{
const char *root_dir, *master_uri, *special_uri;
+ FILE *fp;
root_dir = dav_svn__get_root_dir(r);
master_uri = dav_svn__get_master_uri(r);
@@ -64,6 +112,14 @@
if (root_dir && master_uri) {
const char *seg;
+ const char *action;
+ int is_commit = 0;
+ if (r->method_number == M_MKACTIVITY) {
+ ctx_data *ctx;
+ int ret = get_userdata(&ctx, r);
+ if (ret == OK)
+ ctx->mk_activity_has_happened = 1;
+ }
/* We know we can always safely handle these. */
if (r->method_number == M_REPORT ||
@@ -71,6 +127,18 @@
return OK;
}
+ action = apr_table_get(r->headers_in, "SVN-ACTION");
+ if (action) {
+ if (strcmp(action, "commit") == 0)
+ is_commit = 1;
+ } else {
+ ctx_data *ctx;
+ int ret = get_userdata(&ctx, r);
+ if (ret == OK && ctx->mk_activity_has_happened == 1)
+ is_commit = 1;
+ }
+
+
/* These are read-only requests -- the kind we like to handle
ourselves -- but we need to make sure they aren't aimed at
working resource URIs before trying to field them. Why?
@@ -79,12 +147,18 @@
of the repository isn't guaranteed to have on hand. */
if (r->method_number == M_PROPFIND ||
r->method_number == M_GET) {
- seg = ap_strstr(r->unparsed_uri, root_dir);
- if (seg && ap_strstr_c(seg,
- apr_pstrcat(r->pool, special_uri,
- "/wrk/", NULL))) {
- seg += strlen(root_dir);
- proxy_request_fixup(r, master_uri, seg);
+ if (is_commit) {
+ seg = ap_strstr(r->unparsed_uri, root_dir);
+ if (seg && (ap_strstr_c(seg,
+ apr_pstrcat(r->pool, special_uri,
+ "/wrk/", NULL)) ||
+ ap_strstr_c(seg,
+ apr_pstrcat(r->pool, special_uri,
+ "/vcc/", NULL))
+ )) {
+ seg += strlen(root_dir);
+ proxy_request_fixup(r, master_uri, seg);
+ }
}
return OK;
}
