[
https://issues.apache.org/jira/browse/DERBY-3162?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12573747#action_12573747
]
Øystein Grøvlen commented on DERBY-3162:
----------------------------------------
I have looked a bit closer at the ReplicationRun class, and I have
some comments. Note, however, as I said in my previous comment, if
you like, I can commit the existing patch and you can work on the
improvements in follow-up patches. Some comments:
- I must admit that it took me some time to understand how I should
go about to add new tests, but if I understand it correctly, the
way to do it is to subclass ReplicationRun for my test class and
use the methods of ReplicationRun to initiate replication. I miss
some instructions on how to go about writing new tests in this
framework.
- I feel it is a bit confusing that ReplicationRun seems to be both a
framework and a test. I think it would be better if the methods
used to perform replication operations were separated from actual
test cases.
- I also think it would be a good idea to separate the code for
testing with localhost from the code for testing with master and
slaves on different hosts. In other words, I suggest having two
classes (with a common base class), one for local testing and one
for distributed testing both implementing the same set of methods
like startServer, initMaster etc.
- I miss some documentation of all the static fields in
ReplicationRun.
- Many of the fields are both initialized when they are declared and
in initEnvironment. It seems a bit errorprone to duplicate this.
- Why is masterServerHost etc. set to localhost in ReplicationRun? I
thought that was supposed to be behavior that were specific to
ReplicationRun_Local.
- I think the classes Utils and State should be given more specific
names.
- I think it is a source of confusion that both ReplicationRun and
its inner class State contains a method called initEnvironment. By
the way, the State does not currently seem to be in use.
- Some unused imports: FileChannel, Inet6Address, JDBC
- I am not sure it is a good idea to import whole packages. At
least for java.util, Properties seems to be the only class used.
Why not import just that class.
- The freezeDB string is empty. Note that you still need to freeze
the database when replication is started.
- Some comments have references to PoC. I guess that is obsolete.
- Does masterServer and slaveServer need to be declared at class
level? They seem to be local to the test case when they are used.
- Why implement setUp and tearDown when they only call the
corresponding method in the super class?
- The file name README.properties is a bit awkward since IDEs and
other tools may think it is a properties file, which it is not.
> Create a framework for replication tests
> ----------------------------------------
>
> Key: DERBY-3162
> URL: https://issues.apache.org/jira/browse/DERBY-3162
> Project: Derby
> Issue Type: Sub-task
> Components: Test
> Affects Versions: 10.4.0.0
> Reporter: Ole Solberg
> Assignee: Ole Solberg
> Priority: Minor
> Attachments: derby-3162.1-v1.diff.txt, derby-3162.2-v2.diff.txt,
> derby-3162.2-v2.stat.txt
>
>
> Handle
> - starting and stopping Derby servers to have the master and slave
> replication roles,
> - doing administrative commands like startreplication, startslave,
> stopreplication, failover,
> - performing consistency checks on the slave vs. the master,
> - running load clients against master and slave in the various states of
> replication,
> - provoking error situations on master and slave, and network,
> - ...
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
