>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.
The current code is not used in any part of the codebase for now.
I find it difficult to factor in tests which would use the code.
I humbly request that this code be allowed to remain for now and
promise to write comprehensive tests that shall cover all the
capabilities of this code once the features that use this code are
>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.
Modified to sending SQLState instead of generic message.
>I would like to see the object type defined (e.g., LogRecord, String)
>for each message type.
// This flag is used to send an acknowledgment of successful completion
// of a requested operation. It will however not be used to signify
// reception for every message transmission. The object this message
// type contains will be a <code>String</code>.
public static final int TYPE_ACK = 1;
>So the slave will not use this code to listen for messages?
No the communication will only be through the streams obtained from
the socket after the initial connection has been established.
>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?
You are correct. I have removed the check in readExternl.
>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
Since I now use a new class that contains all the attributes and
initializations common to the network classes I have allowed this to
remain in the common initializer class. I will remove this if it is felt
that this is not OK.
>Yes, I think the comment is a bit misleading.(related to comment no 9)
I have removed this.
>What about the comment in the catch block? Cannot an error also happen
>in the initialization of ObjectOutputStream?
fixed this comment.
>1. ReplicationMessageTransmit and ReplicationMessageReceive has much
> in common, but the organization of the code differs in several
> places. Examples:
> - The ...Transmit constructor calls a private function which
> does the necessary work while ...Receive has much of the code
> in the constructor.
> - ...Tramsmit has a separate class for the privileged operation,
> ...Receive does not.
> I think it would be better if the code was more uniform. Maybe
> some of it could even be shared? (E.g., member variables and code
> to resolve defaults.)
I have factored the common attributes and methods into a class
>2. Only part of the code in the ReplicationMessage... classes is
> particular to replication. For example,
> ReplicationMessageTransmit#sendAndReceiveAck is the only method in
> that class that has any replication specific code. I think you
> should consider to factor out the replication specific code to
> separate classes that use your Transmit and Receive classes.
The transmit class is now broken into
ReplicationClientSocketUtil - That contains the Network related methods
ReplicationMessageTransmit - That handles the actual transmission of the
messages and the setting up of the connection.
I could not do the same with ReplicationMessageReceive because the receive class
actually would be receiving on the client socket and then on the Input Stream obtained
from this socket. Hence now we would have once class that listens on a socket upon
receiving a message relinquishes control to another which would handle this message
and again take back control from this class.
We cannot have multi-threading here because we need the log records to be written in the
log file in the same order in which they are received. If we use threading we would need
to schedule these threads appropriately which would again become cumbersome.
>3. It is not clear to me what the user contract is for the
> ReplicationMessage... classes. I think you need to add Javadoc
> that describe how they are supposed to be used, and how they behave
> in the normal case (e.g., it needs to be mentioned that
> ReplicationMessageReceive#receive does not return until an error
> occurs) and in a failure situation (e.g., will they try to
> reconnect automatically?).
I have added method specifics to the javadocs of all the related methods and shall add
more information if you think this is not sufficient.