[jira] [Updated] (KAFKA-351) Refactor some new components introduced for replication

["Jun Rao (JIRA)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/KAFKA-351?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jun Rao updated KAFKA-351:
--------------------------

    Attachment: kafka-351_v1.patch

Attach patch v1. An overview of the patch.
A. Use synchronized instead of lock for synchronization. The latter has more functionality, but more overhead. See
http://blog.rapleaf.com/dev/2011/06/16/java-performance-synchronized-vs-lock/

B. Partition: Consolidated all reads/writes to leader/ISR in Partition and all accessed are synchronized. This makes sure that leader/ISR values don't change while doing the following operations.
- makeLeader
- makerFollower
- maybeShrinkISR
- updateLeaderHWAndMaybeExpandISR (for maintaining remote replicas)
- checkEnoughReplicasReachAnOffset (for checking if a produce request is satisfied)
This means that Partition has to access ReplicaManager. In some sense, Partition probably should be a nested class under ReplicaManager since it needs to access several members of ReplicaManager. However, this will make ReplicaManager too big.

C. RepicaManager:
- Moved most per partition operations to Partition.
- Cleaned up public methods and added a few helper methods for getting partition and replica.
- Use ConcurrentHashMap for managing all partitions.

D. KafkaApis: Removed callbacks in the constructer. Instead, call methods in ReplicaManager directly.

E. Replica:
- Changed to the new getter/setter style for logEndOffset and highWatermark.
- Local replica doesn't need to set logEndOffset anymore since the logEndOffsetUpdateTime for local replica is never used.

F. BrokerPartitionInfo: Partition is now a complex structure and is supposed to be used only on the server side. Create a simpler PartitionAndLeader class for client usage.

G. KafkaZookeeper: Removed ensurePartitionLeaderOnThisBroker(). The checking of the existence of a leader is now done by replicaManager.leaderReplicaIfLocalOrException(), which is cheaper since it doesn't access ZK.

H. ISRExpirationTest: Remove the test testISRExpirationForMultiplePartitions. It doesn't seem to add additional value since ISR expiration is always done on a per partition basis.

I. SyncProducerTest: Remove the test testProducRequestForUnknowTopic since the logic is always covered by #1 in testProduceCorrectlyReceivesResponse.

J. TestUtils: Added a helper method leaderLocalOnBroker() that more reliably ensures that a leader exists on a broker.

K. TopicCounTest: removed since it's not doing any useful test.

L. ZookeeperConsumerConnectorTest.testCompressionSetConsumption(): removed the part that tests consumer timeout since it's covered in testBasic already.

                
> Refactor some new components introduced for replication 
> --------------------------------------------------------
>
>                 Key: KAFKA-351
>                 URL: https://issues.apache.org/jira/browse/KAFKA-351
>             Project: Kafka
>          Issue Type: Bug
>    Affects Versions: 0.8
>            Reporter: Neha Narkhede
>            Assignee: Jun Rao
>         Attachments: kafka-351_v1.patch
>
>
> Jay had some good refactoring suggestions as part of the review for KAFKA-46. I'd like to file this umbrella JIRA with individual sub tasks to cover those suggestions

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira