Mailing-List: contact derby-dev-help@db.apache.org; run by ezmlm
Precedence: bulk
Reply-To: <derby-dev@db.apache.org>
Message-ID: <24924564.1188912646513.JavaMail.jira@brutus>
Date: Tue, 4 Sep 2007 06:30:46 -0700 (PDT)
From: =?utf-8?Q?=C3=98ystein_Gr=C3=B8vlen_=28JIRA=29?= <jira@apache.org>
To: derby-dev@db.apache.org
Subject: [jira] Commented: (DERBY-2921) Replication: Add a network service
 that connects the master and slave Derby instances
In-Reply-To: <5170718.1184151365366.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-2921?page=3Dcom.atlassian=
.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12524721 ]=20

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

V.Narayanan (JIRA) wrote:
> I have addressed comments 4-19 and am working=20
> on the comments 1, 2 and 3. I will address 1, 2 and 3
> and post a follow up again.
>=20

Thanks you for your quick response to my comments.  Generally, the
changes look good.  I have a few follow-up comments below.

> -18
>=20
> * Will it be OK if I write unit test in a follow-up patch to this issue. =
This
>   code is not being used for now and I feel it is harmless for now.

That is certainly OK.  I think that by writing tests you get
experience in using the interface you have created, and you may
realize that parts of interface should be changed.  Hence, it is good
to write the tests before too much code has been written that rely on
the current interface.

>=20
> -17
>=20
> * For now TYPE_ERROR messages are used only during the initial
>   exchange while exchanging UID versions. This did not contain a
>   SQLState to be transmitted.  I think this is a good idea and I
>   will use this when a SQLException is actually thrown.

Currently, you always generate a specific SQLState
(REPLICATION_MASTER_SLAVE_VERSION_MISMATCH) on the master when you get
any error on the initial message.  Instead, this SQLState could be
sent by the slave when it detects a version mismatch.

>=20
> -16
>=20
> * I take this comment as to mean that the javadoc should explicitly
>   define what kind of object needs to be transmitted as part of this
>   ReplicationMessage. I will update the javadoc to indicate what
>   type flag corresponds to what type of message. I shall also update
>   the type flag definitions to define what type of message each of
>   them signify
>=20
>   For example
>=20
>   public static final int TYPE_LOG =3D 0;
>=20
>   will be modified to
>=20
>   //This flag will be used for all messages will carry LogRecords.
>   public static final int TYPE_LOG =3D 0;

I would like to see the object type defined (e.g., LogRecord, String)
for each message type.

> - 14
>=20
> * ReplicationMessageReceive javadoc: "receives the message from=20
>   the slave ...".  Is this class only to be used by the master?
>=20
>   Although the class might serve a more generic purpose after the suggest=
ed
>   factoring out of the generic network related code and the replication c=
ode
>   in the current context the comment is correct since the intention was t=
o use
>   it only as a receiver of messages on the master.

So the slave will not use this code to listen for messages? =20

>=20
> - 13
>=20
> * I planned to use the version check in
>   ReplicationMessage#readExternal at a later stage to be able to
>   handle a heterogenous collection of Derby version code.
>=20
>   I did not want to do version checking here during initiation
>   because I need to be able to inform the master that it suffers
>   from an incompatibility with the slave. I do not want to abruptly
>   fail the slave without informing the master and if I start to
>   inform the master for every message received it would be too much
>   communication overload.
>=20
>   I thought this initiator message would allow me to establish
>   message incompatibilities at startup and also

But if there is a mismatch, the check in readExternal will throw an
IOException before you are able to inspect the content of the message.
Or have I misunderstood something?


> - 11
>=20
> - 10
>=20
> * I have changed ReplicationMessageReceive to accept a hostName. So
>   this would take care of both 11 and 10.

Looks good, but note that the getLocalHost in defaultNetworkValues is
not necessary anymore since AFAIK this will have already been done
when getByName is called by the constructor and the supplied hostName
is null.

>=20
> - 9
>=20
> * Since the underlying code will throw this exception do you want me
>   to change this comment?

Yes, I think the comment is a bit misleading.


> - 7
>=20
> * Modified the javadoc to mention both ObjectInputStream and ObjectOuputS=
tream

What about the comment in the catch block?  Cannot an error also happen
in the initialization of ObjectOutputStream?


> Replication: Add a network service that connects the master and slave Der=
by instances
> -------------------------------------------------------------------------=
------------
>
>                 Key: DERBY-2921
>                 URL: https://issues.apache.org/jira/browse/DERBY-2921
>             Project: Derby
>          Issue Type: Sub-task
>          Components: Services
>    Affects Versions: 10.4.0.0
>            Reporter: J=C3=B8rgen L=C3=B8land
>            Assignee: V.Narayanan
>         Attachments: Replication_Network_v1.diff, Replication_Network_v1.=
stat, Replication_Network_v2.diff, Replication_Network_v2.stat, Replication=
_Network_v3.diff, Replication_Network_v3.stat, Replication_Network_v4.diff,=
 Replication_Network_v4.stat
>
>
> A network connection is required between the master and slave Derby insta=
nces of a replicated database. The connection will be used to send many kin=
ds of messages, including:
> * log records
> * the database (when replication is started)
> * master -> slave commands (like "stop replication")

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