On Wed, Aug 31, 2016 at 07:32:41PM -0700, Andy Zhou wrote:
> Added the '--no-sync' option base on feedbacks of current
> implementation.
>
> Added appctl command "ovsdb-server/sync-status" based on feedbacks
> of current implementation.
>
> Added the RPL_S_INIT state in the state machine. This state is
> not strictly necessary for the replication state machine, but is
> introduced to make sure the state is update immediately when
> the state machine is reset, via replication_init(). Without it
> ovsdb/sync-status may display "replicating" or crash, if the command
> is issued between after replication_init() is called, but before
> the state variable is updated from replication_run().
>
> Added a test to simulate the integration of HA manager with OVSDB
> server using replication.
>
> Other documentation and API improvements.
>
> Tested-by: Numan Siddique <nusid...@redhat.com>
> Signed-off-by: Andy Zhou <az...@ovn.org>
Thanks for working on this.
Until I read the documentation fairly carefully, I was a bit mystified
by --no-sync. I don't like negative forms very much to start out with,
but it also wasn't clear why I'd want to specify where to sync from but
not to sync. Then I figured out that it's really just saying to start
in active mode.
So I'd suggest renaming --no-sync to --active.
Here's a suggested incremental.
Acked-by: Ben Pfaff <b...@ovn.org>
--8<--------------------------cut here-------------------------->8--
diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
index 29092e0..c33ab86 100644
--- a/ovsdb/ovsdb-server.c
+++ b/ovsdb/ovsdb-server.c
@@ -273,10 +273,10 @@ main(int argc, char *argv[])
fatal_ignore_sigpipe();
process_init();
- bool no_sync = false;
+ bool active = false;
parse_options(&argc, &argv, &remotes, &unixctl_path, &run_command,
- &sync_from, &sync_exclude, &no_sync);
- is_backup = sync_from && !no_sync;
+ &sync_from, &sync_exclude, &active);
+ is_backup = sync_from && !active;
daemon_become_new_user(false);
@@ -1488,7 +1488,7 @@ ovsdb_server_get_sync_status(struct unixctl_conn *conn,
int argc OVS_UNUSED,
static void
parse_options(int *argcp, char **argvp[],
struct sset *remotes, char **unixctl_pathp, char **run_command,
- char **sync_from, char **sync_exclude, bool *no_sync)
+ char **sync_from, char **sync_exclude, bool *active)
{
enum {
OPT_REMOTE = UCHAR_MAX + 1,
@@ -1498,7 +1498,7 @@ parse_options(int *argcp, char **argvp[],
OPT_PEER_CA_CERT,
OPT_SYNC_FROM,
OPT_SYNC_EXCLUDE,
- OPT_NO_SYNC,
+ OPT_ACTIVE,
VLOG_OPTION_ENUMS,
DAEMON_OPTION_ENUMS
};
@@ -1519,7 +1519,7 @@ parse_options(int *argcp, char **argvp[],
{"ca-cert", required_argument, NULL, 'C'},
{"sync-from", required_argument, NULL, OPT_SYNC_FROM},
{"sync-exclude-tables", required_argument, NULL, OPT_SYNC_EXCLUDE},
- {"no-sync", no_argument, NULL, OPT_NO_SYNC},
+ {"active", no_argument, NULL, OPT_ACTIVE},
{NULL, 0, NULL, 0},
};
char *short_options = ovs_cmdl_long_options_to_short_options(long_options);
@@ -1594,8 +1594,8 @@ parse_options(int *argcp, char **argvp[],
*sync_exclude = xstrdup(optarg);
break;
}
- case OPT_NO_SYNC:
- *no_sync = true;
+ case OPT_ACTIVE:
+ *active = true;
break;
case '?':
diff --git a/ovsdb/replication.c b/ovsdb/replication.c
index 40a9fd0..c2f9dfb 100644
--- a/ovsdb/replication.c
+++ b/ovsdb/replication.c
@@ -906,10 +906,9 @@ replication_usage(void)
{
printf("\n\
Syncing options:\n\
- --sync-from=SERVER sync DATABASE from active SERVER\n\
- start in backup mode, except when\n\
- --no-sync is also specified\n\
+ --sync-from=SERVER sync DATABASE from active SERVER and start in\n\
+ backup mode (except with --active)\n\
--sync-exclude-tables=DB:TABLE,...\n\
exclude the TABLE in DB from syncing\n\
- --no-sync start in active mode\n");
+ --active with --sync-from, start in active mode\n");
}
diff --git a/ovsdb/replication.man b/ovsdb/replication.man
index be827b9..a2ea665 100644
--- a/ovsdb/replication.man
+++ b/ovsdb/replication.man
@@ -2,18 +2,22 @@ The following options allow \fBovsdb\-server\fR to
synchronize its databases
with another running \fBovsdb\-server\fR.
.TP
\fB\-\-sync\-from=\fIserver\fR
-Causes \fBovsdb\-server\fR to synchronize its databases with the databases in
-\fIserver\fR. Every transaction committed by \fIserver\fR will be replicated
-to \fBovsdb\-server\fR. \fIserver\fR is an active connection method in one of
-the forms documented in \fBovsdb\-client(1)\fR.
-However if the \fB\-\-no\-sync\fR is also specified, replication will be
-postponed until \fBovs-appctl(1)\fR command
-\fBovsdb\-server/connect\-active\-ovsdb\-server\fR is issued.
+Sets up \fBovsdb\-server\fR to synchronize its databases with the
+databases in \fIserver\fR, which must be an active connection method
+in one of the forms documented in \fBovsdb\-client\fR(1). Every
+transaction committed by \fIserver\fR will be replicated to
+\fBovsdb\-server\fR. This option makes \fBovsdb\-server\fR start
+as a backup server; add \fB\-\-active\fR to make it start as an
+active server.
.TP
\fB\-\-sync\-exclude-tables=\fIdb:table[,db:table]...\fR
Causes the specified tables to be excluded from replication.
.TP
-\fB\-\-no\-sync\fR
-Always start the server as an active server. Use this option to allow
-the syncing options to be specified using command line options, yet start
-the server, as the default, active server.
+\fB\-\-active\fR
+By default, \fB\-\-sync\-from\fR makes \fBovsdb\-server\fR start up as
+a backup for \fIserver\fR. With \fB\-\-active\fR, however,
+\fBovsdb\-server\fR starts as an active server. Use this option to
+allow the syncing options to be specified using command line options,
+yet start the server, as the default, active server. To switch the
+running server to backup mode, use \fBovs-appctl(1)\fR to execute the
+\fBovsdb\-server/connect\-active\-ovsdb\-server\fR command.
diff --git a/tests/ovsdb-server.at b/tests/ovsdb-server.at
index e7daefd..89a5bf9 100644
--- a/tests/ovsdb-server.at
+++ b/tests/ovsdb-server.at
@@ -1308,7 +1308,7 @@ dnl Start both 'db1' and 'db2' in backup mode. Let them
backup from each
dnl other. This is not an supported operation state, but to simulate a start
dnl up condition where an HA manger can select which one to be an active
dnl server soon after.
-AT_CHECK([ovsdb-server --detach --no-chdir --log-file=ovsdb-server1.log
--pidfile="`pwd`"/pid --remote=punix:db.sock --unixctl="`pwd`"/unixctl db1
--sync-from=unix:db2.sock --no-sync ], [0], [ignore], [ignore])
+AT_CHECK([ovsdb-server --detach --no-chdir --log-file=ovsdb-server1.log
--pidfile="`pwd`"/pid --remote=punix:db.sock --unixctl="`pwd`"/unixctl db1
--sync-from=unix:db2.sock --active ], [0], [ignore], [ignore])
on_exit 'test ! -e pid || kill `cat pid`'
AT_CHECK([ovs-appctl -t "`pwd`"/unixctl
ovsdb-server/connect-active-ovsdb-server])
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
