Mailing-List: contact derby-dev-help@db.apache.org; run by ezmlm
Precedence: bulk
Reply-To: <derby-dev@db.apache.org>
Message-ID: <25268832.1201855868490.JavaMail.jira@brutus>
Date: Fri, 1 Feb 2008 00:51:08 -0800 (PST)
From: =?utf-8?Q?=C3=98ystein_Gr=C3=B8vlen_=28JIRA=29?= <jira@apache.org>
To: derby-dev@db.apache.org
Subject: [jira] Commented: (DERBY-3205) Replication: Add connection url
 command options for starting, stopping slave and for failover
In-Reply-To: <20606458.1195128343797.JavaMail.jira@brutus>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable


    [ https://issues.apache.org/jira/browse/DERBY-3205?page=3Dcom.atlassian=
.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=3D1256=
4686#action_12564686 ]=20

=C3=98ystein Gr=C3=B8vlen commented on DERBY-3205:
----------------------------------------

Thanks for the patch, J=C3=B8rgen.  It looks good, and most of my comments
are minor.  If you like, I can  commit the current version of the
patch, and you can handle my comments in a follow-up patch.

To my comments:

1. EmbedConnection:=20

   a. handleDBNotFound(): The comment "the finally clause below" is
      not right anymore.  Also, the finally clause referred to is only
      in one of the calling methods.

   b. handleStopReplicationSlave(): The two cases makes me wonder
      whether this should rather be two methods.  Javadoc:  "call case
      2)" sounds a bit strange.  The case 2) description ends with an
      incomplete sentence.

2. SlaveDatabase:

   a. Unused imports of MessageId and ReplicationLogger

   b. Does not slaveFac need to be volatile?  The synchronization in
      setSlaveFactory() does not seem to be of any help since other
      access is not synchronized.

   c. stop():  I would like a comment explaining when and for what
      purpose this method is called.

   d. verifyShutdownSlave(): Why does this method need to be
      synchronized when shutdownInitiated is volatile?

   e. handleShutdown(): Seems a bit strange to use the external JDBC
      API in an internal class.  Is there not some method that can be
      called to shut down a Database directly?  If doing this at JDBC
      level, I wonder whether it would be better use
      EmbeddedDataSource instead of using the DriverManager.

3. SlaveController:

   a. When I experiment with stopSlave, I see that the slave shutdown
      message is written to derby.log twice.  I have not looked into
      why, but I think once should be sufficient :-)

   b. stop(): Javadoc says that this should be called after either
      stopSlave and failover, but I think this method is used by
      Monitor when stopping the services of a system, and I am not
      sure you can control the shutdown sequence for the services.

   c. startSlave(): The assumption that if it is not LogToFile, it
      must be ReadOnly is right today.  However, this assumption makes
      the code brittle since it is will be hard to detect for people
      that want to add new implementations of LogFactory.

   d. stopSlave(): Javadoc still says it is not implemented yet.

   e. stopSlave(boolean): Javadoc: "database system shutdown".  Do you
      mean the shutdown of this database, or the entire system by this
      term.  For the explanation of forceStop, I would rather say that
      "if true, the slave will be shut down even if connected to the
      master", or something.  (Note: Same javadoc in SlaveFactory.
      Maybe you should consider to use @see instead?)

4. LogToFile:=20

   a. AFAICT when the slave is stopped, recovery thread will complete
      the redo of the current log file before stopping.  Is that
      intentional?

   b. stopReplicationSlaveMode() and stopReplicationSlaveRole() is
      confusingly similar names.  At least, it confused me.  I am not
      sure of the distinction between mode and role here, and but I do
      not think the names reflect that the first causes the slave to
      stop while the latter causes it to do failover.

5. messages:

   a. Generally, it seems like the names of several of the SQLState
      constants are more general that the associated text.  That can
      lead people to reuse constants when the message is not really
      appropriate.

   b. XRE40: Would it be an idea to mention the database name here?

   c. XRE41: Maybe the message could say more explicitly that the
      operation was requested on a slave database.

   d. XRE43: First sentence is general, but the second sentence seems
      to assume a very specific context.
     =20


> Replication: Add connection url command options for starting, stopping sl=
ave and for failover
> -------------------------------------------------------------------------=
--------------------
>
>                 Key: DERBY-3205
>                 URL: https://issues.apache.org/jira/browse/DERBY-3205
>             Project: Derby
>          Issue Type: Sub-task
>          Components: JDBC
>    Affects Versions: 10.4.0.0
>            Reporter: J=C3=B8rgen L=C3=B8land
>            Assignee: V.Narayanan
>         Attachments: derby-3205_startslave_dontcommit.diff, derby-3205_st=
artslave_dontcommit.stat, failover_impl_3205_NotForCommit_v1.diff, failover=
_impl_3205_NotForCommit_v1.stat, failover_impl_3205_v1.diff, failover_impl_=
3205_v1.stat, failover_impl_3205_v2.diff, failover_impl_3205_v2.stat, Socke=
t_Close_Fix_v1.diff, Socket_Close_Fix_v1.stat, startSlave_1a.diff, startSla=
ve_1a.stat, startSlave_1b.diff, startSlave_1b.stat, startSlave_1c.diff, sta=
rtSlave_1d.diff, StopSlave_impl_3205_NotForCommit_v1.diff, StopSlave_impl_3=
205_NotForCommit_v1.stat, stopSlave_v1a.diff, stopSlave_v1a.stat, stopSlave=
_v1b.diff, stopSlave_v1b.stat
>
>
> Add commands to start and stop the replication slave using properties or =
connection url. Example:
> 'jdbc:derby:<host><dbname>;startSlave=3Dtrue';
> 'jdbc:derby:<host><dbname>;stopSlave=3Dtrue';
> 'jdbc:derby:<host><dbname>;failover=3Dtrue;
> Connection url options that must be recognized:
> startSlave=3Dtrue
> stopSlave=3Dtrue
> failover=3Dtrue
> slaveHost=3D<host> (optional, defaults to localhost) (only for startSlave=
)
> slavePort=3D<port> (optional, defaults to 8001) (only for startSlave)
> See functional specification on Derby-2872 for further details.

--=20
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.