You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Christopher Freeman <cf...@linkedin.com> on 2013/09/06 22:04:20 UTC

Re: Review Request 14013: Added support for Scala 2.10 builds while maintaining compatibility with 2.8.x

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14013/
-----------------------------------------------------------

(Updated Sept. 6, 2013, 8:04 p.m.)


Review request for kafka and Neha Narkhede.


Changes
-------

added kafka to group
jira ticket is KAFKA-1046


Bugs: KAFKA-1046
    https://issues.apache.org/jira/browse/KAFKA-1046


Repository: kafka


Description
-------

This RB contains necessary changes to the kafka source code to support Scala 2.10.x while still maintaining support for Scala 2.8.x

The existing cause of source incompatibility between 2.8.x and 2.10.x was primarily due to the use of implicit conversions in the scala.collection.JavaConversions object.  Many of them where deprecated (and replaced with differently named methods) and the deprecated methods where removed in 2.10.  I avoided the source incompatibility by importing the implicits with a wild card.

Scala annotations moved to a different package in 2.9 and the reference to the old location was removed in 2.10.  I couldn't think of a source compatible way to support both in Annotations.scala so since this source file is very small, I created two copies and configured sbt to use the one appropriate for when building for a particular Scala version.

I also typed many of the catch block case statements with :Throwable.  This change isn't required but starting with Scala 2.9, the compiler will emit warnings if the type is emitted.  Otherwise "case e =>" and case "e: Throwable =>" are equivalent.


  


Diffs
-----

  core/build.sbt c54cf443a5e22f86b2981ff1eb535467a5648739 
  core/src/main/scala/kafka/Kafka.scala dafb1ee78f2f28616e82d208692120350e9898b4 
  core/src/main/scala/kafka/admin/AdminUtils.scala 9ce89cd561a6bae94361c61aee7b9bd5093b2442 
  core/src/main/scala/kafka/admin/DeleteTopicCommand.scala 3da4518f5834bcc7574a83a492b6629dfca26a39 
  core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala 53fc433c7bd80c4f09cf69cbba25443adc708364 
  core/src/main/scala/kafka/admin/ReassignPartitionsCommand.scala 8d287f498b0061d6fa38268ec00de090f7cca516 
  core/src/main/scala/kafka/client/ClientUtils.scala cc526ec933052b239f0e7ce43e76cd9d011d5bd9 
  core/src/main/scala/kafka/cluster/Broker.scala 435c47348eb5889cd1f416ebbf874ab33a632741 
  core/src/main/scala/kafka/consumer/ConsoleConsumer.scala 140f2e33522803a67bc79d53f547b7cd15f11468 
  core/src/main/scala/kafka/consumer/ConsumerFetcherManager.scala 71ae640de202b54e1ea8d25d6b70d6078a8df0c5 
  core/src/main/scala/kafka/consumer/SimpleConsumer.scala 1c4e1da09a37c6a0fdc13bfb81e3ac2b529e4acc 
  core/src/main/scala/kafka/consumer/TopicCount.scala c8e84065826cdedeca48ae92dec463c6e8569838 
  core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala e3a64204513467cef8917f501c3bc0e5b1db2e3e 
  core/src/main/scala/kafka/consumer/ZookeeperTopicEventWatcher.scala df83baa050a02d6171b5d182ef80de4536304441 
  core/src/main/scala/kafka/controller/ControllerChannelManager.scala 38b867467bb2b5f9ece055c95f251e65aaad2fd1 
  core/src/main/scala/kafka/controller/KafkaController.scala 5ac38fd697695ade44727ff6980df4540aa4e042 
  core/src/main/scala/kafka/controller/PartitionStateMachine.scala deebed0a5d65b94c1bf24e92ebc7d031dff8cbc2 
  core/src/main/scala/kafka/controller/ReplicaStateMachine.scala 0c9d43602a38a5878d11c86979f0289c679734f7 
  core/src/main/scala/kafka/javaapi/FetchRequest.scala 83d8cbc97fafbf4edcc9139260efa971edb57c76 
  core/src/main/scala/kafka/javaapi/Implicits.scala 9a639140ca2f8151dcdfc40ac1eb32711db54598 
  core/src/main/scala/kafka/javaapi/OffsetCommitRequest.scala 32033d6a6634f33a48ef2b03734d45434b51ad8b 
  core/src/main/scala/kafka/javaapi/OffsetCommitResponse.scala d1c50c46cf74b080eada293dc0023c24e437def5 
  core/src/main/scala/kafka/javaapi/OffsetFetchRequest.scala 64d134bec86e01387e32b6ee7882b5902544df9c 
  core/src/main/scala/kafka/javaapi/OffsetFetchResponse.scala 9f83c1be65590fe019d679d5d703aaf4f53792c5 
  core/src/main/scala/kafka/javaapi/OffsetRequest.scala 3565a154e5269680c62baef06094418647f36648 
  core/src/main/scala/kafka/javaapi/TopicMetadata.scala 97b6dcdd1db8084f6598a5e2b2c4cc3d66fa219e 
  core/src/main/scala/kafka/javaapi/TopicMetadataRequest.scala 5f80df7bd4dc015434b61f9efeb7ae2260ae6a5c 
  core/src/main/scala/kafka/javaapi/consumer/ZookeeperConsumerConnector.scala 14c4c8ad0861694cad5b69f703d4dac1f55a2163 
  core/src/main/scala/kafka/javaapi/message/ByteBufferMessageSet.scala 0a952480b30377cdd394f1358a7ddb7d2683970f 
  core/src/main/scala/kafka/javaapi/producer/Producer.scala 72653285607783577fc2a04480f00a1882312f22 
  core/src/main/scala/kafka/log/Log.scala 626eb8f48a1d2c44ecafa32242bd17c0a2c686ff 
  core/src/main/scala/kafka/log/LogConfig.scala 48660bcd022cdd08db1168a354209728c695e683 
  core/src/main/scala/kafka/log/LogManager.scala d039f9d4148f7ece93bcbcbd846f3442eac1278e 
  core/src/main/scala/kafka/network/BoundedByteBufferReceive.scala cab18643f64c9261b68fc758b630b39b2c579ccb 
  core/src/main/scala/kafka/producer/SyncProducer.scala 306f200f325f58da915d7d8af1651eef0614b913 
  core/src/main/scala/kafka/producer/async/DefaultEventHandler.scala 48ddb6a358baefde6c008c22577928f1e2d61da9 
  core/src/main/scala/kafka/producer/async/ProducerSendThread.scala 2b41a4996ce4f9b36dbffd73005d755dd4a8fb4f 
  core/src/main/scala/kafka/server/AbstractFetcherThread.scala 7663face23ef58908fcbfd80c8abc303460964fe 
  core/src/main/scala/kafka/server/KafkaApis.scala 0ec031ad9423b82ba9c8a49fe984337620392a8b 
  core/src/main/scala/kafka/server/KafkaServerStartable.scala 5be65e9799f68c200aa7fb377972ff5935e7e14a 
  core/src/main/scala/kafka/server/ReplicaManager.scala 73c87c663981002b52a0c4995a6ef96ca24d5ef4 
  core/src/main/scala/kafka/server/TopicConfigManager.scala 5814cb7c4ee8c11753afb1e85771040f399e6704 
  core/src/main/scala/kafka/server/ZookeeperLeaderElector.scala 574922b690d6a9c84b2552b56a4459dc8c2d003c 
  core/src/main/scala/kafka/tools/ImportZkOffsets.scala 63519e12ed5044dca0f53b0bc1234ba8fa861cd7 
  core/src/main/scala/kafka/tools/JmxTool.scala 7e424e7683ba7ec24e74848b738a1405258c0da3 
  core/src/main/scala/kafka/tools/MirrorMaker.scala 2d939470ee29cd2fcd55389aa71c5c3902d4b6f5 
  core/src/main/scala/kafka/tools/SimpleConsumerShell.scala 7629329dc004e198a34831b40a503ac0133569ab 
  core/src/main/scala/kafka/utils/Annotations.scala 28269eb037109f7680b9da732e4baa51c9a594b6 
  core/src/main/scala/kafka/utils/Annotations_2.8.scala PRE-CREATION 
  core/src/main/scala/kafka/utils/Json.scala 3f1252c28035d7c5065f60987cfb93455403f46e 
  core/src/main/scala/kafka/utils/KafkaScheduler.scala 73457e13f8a83b6faa7510c2f4d0c0bf1b56364e 
  core/src/main/scala/kafka/utils/Mx4jLoader.scala 64d84ccee7df05e7da88930e0a9dd3d375aa632c 
  core/src/main/scala/kafka/utils/Pool.scala 9a86eabb1bba9434d0a13f6b921c281a971bdbc4 
  core/src/main/scala/kafka/utils/VerifiableProperties.scala 9009a9d82cc6aeb098f68cae1bc2608fb7e0d09c 
  core/src/main/scala/kafka/utils/ZkUtils.scala 84744ebaa6342b5b05746c8acc18bd15ac8ec30e 
  core/src/test/scala/unit/kafka/consumer/ZookeeperConsumerConnectorTest.scala 121b6c5079b011d9df8699855fae3345ffd84931 
  core/src/test/scala/unit/kafka/javaapi/consumer/ZookeeperConsumerConnectorTest.scala 9f243f0ab1efdf48c5162a15fba6be976f37b5d2 
  core/src/test/scala/unit/kafka/javaapi/message/BaseMessageSetTestCases.scala abee11b0f2ffe3d392464b92df4a9c73f1318aa0 
  core/src/test/scala/unit/kafka/metrics/KafkaTimerTest.scala fe5bc09e12281176ee71c1c9e10133cfa7dd6518 
  core/src/test/scala/unit/kafka/producer/AsyncProducerTest.scala 74a27430f4e7c99106752f7d21ffe45a26b4b719 
  core/src/test/scala/unit/kafka/producer/ProducerTest.scala 72eccc12ef53af417426a231f925527365624337 
  core/src/test/scala/unit/kafka/producer/SyncProducerTest.scala bbf04063d2be10fe502a180279ee1930cb8ac1ab 
  core/src/test/scala/unit/kafka/utils/TestUtils.scala 10712e27dc1d09ed86a0a54c8e48658daf7c5268 
  project/Build.scala bad93dbd8a45abe21229cb7b3cff295a732a59bb 

Diff: https://reviews.apache.org/r/14013/diff/


Testing
-------

successfully performed cross build across all scala versions
unit tests pass across all scala versions


Thanks,

Christopher Freeman


Re: Review Request 14013: Added support for Scala 2.10 builds while maintaining compatibility with 2.8.x

Posted by Christopher Freeman <cf...@linkedin.com>.

> On Sept. 8, 2013, 10:47 p.m., Jun Rao wrote:
> > I am wondering if there is any performance degradation due to the changes in implicit conversion. Could you run kafka.perf.ProducerPerformance and see if there is any noticeable performance change with and w/o the patch?

both should compile to similar byte code.  Any extra work is done during compile time where the compiler must figure out which method should be invoked rather then being explicitly told by the source code.

Over seven runs of the producer perf test using 1,000,000 messages, for before and after patch, I got 77180.93 and 77350.39 nMsg.sec.  Performance between runs was kind of noisy so the delta (which actually improved) is well within the margin of error.


> On Sept. 8, 2013, 10:47 p.m., Jun Rao wrote:
> > core/src/main/scala/kafka/javaapi/consumer/ZookeeperConsumerConnector.scala, lines 95-97
> > <https://reviews.apache.org/r/14013/diff/3/?file=349538#file349538line95>
> >
> >     Don't you need to specify the java return type explicitly here?

This is not required as the the return type is defined in the trait which the class is extending


> On Sept. 8, 2013, 10:47 p.m., Jun Rao wrote:
> > core/src/main/scala/kafka/controller/PartitionStateMachine.scala, lines 364-365
> > <https://reviews.apache.org/r/14013/diff/3/?file=349531#file349531line364>
> >
> >     Could you move the import of the JavaConversion to within this function to limit the scope of the implicit conversion?

sure.


> On Sept. 8, 2013, 10:47 p.m., Jun Rao wrote:
> > core/src/main/scala/kafka/utils/Annotations_2.9+.scala, lines 1-38
> > <https://reviews.apache.org/r/14013/diff/3/?file=349557#file349557line1>
> >
> >     How does this affect IDEs like Intellij? Are you able to build the project in IDE with both version of threadsafe annotation?

I don't normally build with my IDE but I don't see any errors when I compile the project with Intellij (Build -> Make project, which invokes an external scala compiler).  I also didn't see any issues using the SBT plugin (which makes sense as all it is doing is opening a shell and running sbt).


- Christopher


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14013/#review25979
-----------------------------------------------------------


On Sept. 8, 2013, 9:47 p.m., Christopher Freeman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14013/
> -----------------------------------------------------------
> 
> (Updated Sept. 8, 2013, 9:47 p.m.)
> 
> 
> Review request for kafka and Neha Narkhede.
> 
> 
> Bugs: KAFKA-1046
>     https://issues.apache.org/jira/browse/KAFKA-1046
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> This RB contains necessary changes to the kafka source code to support Scala 2.10.x while still maintaining support for Scala 2.8.x
> 
> The existing cause of source incompatibility between 2.8.x and 2.10.x was primarily due to the use of implicit conversions in the scala.collection.JavaConversions object.  Many of them where deprecated (and replaced with differently named methods) and the deprecated methods where removed in 2.10.  I avoided the source incompatibility by importing the implicits with a wild card.
> 
> Scala annotations moved to a different package in 2.9 and the reference to the old location was removed in 2.10.  I couldn't think of a source compatible way to support both in Annotations.scala so since this source file is very small, I created two copies and configured sbt to use the one appropriate for when building for a particular Scala version.
> 
> I also typed many of the catch block case statements with :Throwable.  This change isn't required but starting with Scala 2.9, the compiler will emit warnings if the type is emitted.  Otherwise "case e =>" and case "e: Throwable =>" are equivalent.
> 
> 
>   
> 
> 
> Diffs
> -----
> 
>   core/build.sbt c54cf44 
>   core/src/main/scala/kafka/Kafka.scala dafb1ee 
>   core/src/main/scala/kafka/admin/AddPartitionsCommand.scala 5757c32 
>   core/src/main/scala/kafka/admin/AdminUtils.scala c399bc7 
>   core/src/main/scala/kafka/admin/CreateTopicCommand.scala 21c1186 
>   core/src/main/scala/kafka/admin/DeleteTopicCommand.scala 3da4518 
>   core/src/main/scala/kafka/admin/ListTopicCommand.scala c760cc0 
>   core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala d5de5f3 
>   core/src/main/scala/kafka/admin/ReassignPartitionsCommand.scala aa61fa1 
>   core/src/main/scala/kafka/client/ClientUtils.scala cc526ec 
>   core/src/main/scala/kafka/cluster/Broker.scala b03dea2 
>   core/src/main/scala/kafka/consumer/ConsoleConsumer.scala 719beb5 
>   core/src/main/scala/kafka/consumer/ConsumerFetcherManager.scala fa6b213 
>   core/src/main/scala/kafka/consumer/SimpleConsumer.scala 4395fe3 
>   core/src/main/scala/kafka/consumer/TopicCount.scala c8e8406 
>   core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala e7a692a 
>   core/src/main/scala/kafka/consumer/ZookeeperTopicEventWatcher.scala df83baa 
>   core/src/main/scala/kafka/controller/ControllerChannelManager.scala ed1ce0b 
>   core/src/main/scala/kafka/controller/KafkaController.scala ab18b7a 
>   core/src/main/scala/kafka/controller/PartitionStateMachine.scala a084830 
>   core/src/main/scala/kafka/controller/ReplicaStateMachine.scala c964857 
>   core/src/main/scala/kafka/javaapi/FetchRequest.scala b475240 
>   core/src/main/scala/kafka/javaapi/Implicits.scala ee0a71d 
>   core/src/main/scala/kafka/javaapi/OffsetRequest.scala 1c77ff8 
>   core/src/main/scala/kafka/javaapi/TopicMetadata.scala 97b6dcd 
>   core/src/main/scala/kafka/javaapi/TopicMetadataRequest.scala 5f80df7 
>   core/src/main/scala/kafka/javaapi/consumer/ZookeeperConsumerConnector.scala 14c4c8a 
>   core/src/main/scala/kafka/javaapi/message/ByteBufferMessageSet.scala 0a95248 
>   core/src/main/scala/kafka/javaapi/producer/Producer.scala 7265328 
>   core/src/main/scala/kafka/log/LogManager.scala 4771d11 
>   core/src/main/scala/kafka/network/BoundedByteBufferReceive.scala cab1864 
>   core/src/main/scala/kafka/producer/SyncProducer.scala 306f200 
>   core/src/main/scala/kafka/producer/async/DefaultEventHandler.scala 2e36d3b 
>   core/src/main/scala/kafka/producer/async/ProducerSendThread.scala 2b41a49 
>   core/src/main/scala/kafka/server/AbstractFetcherThread.scala d5addb3 
>   core/src/main/scala/kafka/server/KafkaApis.scala cd02aab 
>   core/src/main/scala/kafka/server/KafkaServerStartable.scala 5be65e9 
>   core/src/main/scala/kafka/server/ReplicaManager.scala f551243 
>   core/src/main/scala/kafka/server/ZookeeperLeaderElector.scala f1f0625 
>   core/src/main/scala/kafka/tools/ImportZkOffsets.scala 55709b5 
>   core/src/main/scala/kafka/tools/JmxTool.scala 7e424e7 
>   core/src/main/scala/kafka/tools/MirrorMaker.scala 6fb545a 
>   core/src/main/scala/kafka/tools/SimpleConsumerShell.scala 3cfa384 
>   core/src/main/scala/kafka/utils/Annotations.scala 28269eb 
>   core/src/main/scala/kafka/utils/Annotations_2.8.scala PRE-CREATION 
>   core/src/main/scala/kafka/utils/Annotations_2.9+.scala PRE-CREATION 
>   core/src/main/scala/kafka/utils/Json.scala f80b2cc 
>   core/src/main/scala/kafka/utils/Mx4jLoader.scala 64d84cc 
>   core/src/main/scala/kafka/utils/Pool.scala 9a86eab 
>   core/src/main/scala/kafka/utils/Utils.scala e83eb5f 
>   core/src/main/scala/kafka/utils/ZkUtils.scala ca1ce12 
>   core/src/test/scala/unit/kafka/admin/AddPartitionsTest.scala 06be990 
>   core/src/test/scala/unit/kafka/admin/AdminTest.scala dc0013f 
>   core/src/test/scala/unit/kafka/consumer/ZookeeperConsumerConnectorTest.scala fcfc583 
>   core/src/test/scala/unit/kafka/javaapi/consumer/ZookeeperConsumerConnectorTest.scala 9f243f0 
>   core/src/test/scala/unit/kafka/javaapi/message/BaseMessageSetTestCases.scala abee11b 
>   core/src/test/scala/unit/kafka/log/LogTest.scala 4ed88e8 
>   core/src/test/scala/unit/kafka/metrics/KafkaTimerTest.scala fe5bc09 
>   core/src/test/scala/unit/kafka/producer/AsyncProducerTest.scala 1781bc0 
>   core/src/test/scala/unit/kafka/producer/ProducerTest.scala 29331db 
>   core/src/test/scala/unit/kafka/producer/SyncProducerTest.scala b3e89c3 
>   core/src/test/scala/unit/kafka/utils/TestUtils.scala 830608f 
>   perf/src/main/scala/kafka/perf/ConsumerPerformance.scala 3158a22 
>   project/Build.scala b3858f3 
> 
> Diff: https://reviews.apache.org/r/14013/diff/
> 
> 
> Testing
> -------
> 
> successfully performed cross build across all scala versions
> unit tests pass across all scala versions
> 
> 
> Thanks,
> 
> Christopher Freeman
> 
>


Re: Review Request 14013: Added support for Scala 2.10 builds while maintaining compatibility with 2.8.x

Posted by Neha Narkhede <ne...@gmail.com>.

> On Sept. 8, 2013, 10:47 p.m., Jun Rao wrote:
> > core/src/main/scala/kafka/utils/Annotations_2.9+.scala, lines 1-38
> > <https://reviews.apache.org/r/14013/diff/3/?file=349557#file349557line1>
> >
> >     How does this affect IDEs like Intellij? Are you able to build the project in IDE with both version of threadsafe annotation?
> 
> Christopher Freeman wrote:
>     I don't normally build with my IDE but I don't see any errors when I compile the project with Intellij (Build -> Make project, which invokes an external scala compiler).  I also didn't see any issues using the SBT plugin (which makes sense as all it is doing is opening a shell and running sbt).

I tried compiling the code in IntelliJ and it complains about the some errors while compiling Annotations_2.9+.scala. I attached the screenshot on the JIRA - https://issues.apache.org/jira/secure/attachment/12602162/Screen%20Shot%202013-09-09%20at%209.34.09%20AM.png. Do you know of a way to fix this?


- Neha


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14013/#review25979
-----------------------------------------------------------


On Sept. 9, 2013, 12:40 a.m., Christopher Freeman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14013/
> -----------------------------------------------------------
> 
> (Updated Sept. 9, 2013, 12:40 a.m.)
> 
> 
> Review request for kafka and Neha Narkhede.
> 
> 
> Bugs: KAFKA-1046
>     https://issues.apache.org/jira/browse/KAFKA-1046
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> This RB contains necessary changes to the kafka source code to support Scala 2.10.x while still maintaining support for Scala 2.8.x
> 
> The existing cause of source incompatibility between 2.8.x and 2.10.x was primarily due to the use of implicit conversions in the scala.collection.JavaConversions object.  Many of them where deprecated (and replaced with differently named methods) and the deprecated methods where removed in 2.10.  I avoided the source incompatibility by importing the implicits with a wild card.
> 
> Scala annotations moved to a different package in 2.9 and the reference to the old location was removed in 2.10.  I couldn't think of a source compatible way to support both in Annotations.scala so since this source file is very small, I created two copies and configured sbt to use the one appropriate for when building for a particular Scala version.
> 
> I also typed many of the catch block case statements with :Throwable.  This change isn't required but starting with Scala 2.9, the compiler will emit warnings if the type is emitted.  Otherwise "case e =>" and case "e: Throwable =>" are equivalent.
> 
> 
>   
> 
> 
> Diffs
> -----
> 
>   core/build.sbt c54cf44 
>   core/src/main/scala/kafka/Kafka.scala dafb1ee 
>   core/src/main/scala/kafka/admin/AddPartitionsCommand.scala 5757c32 
>   core/src/main/scala/kafka/admin/AdminUtils.scala c399bc7 
>   core/src/main/scala/kafka/admin/CreateTopicCommand.scala 21c1186 
>   core/src/main/scala/kafka/admin/DeleteTopicCommand.scala 3da4518 
>   core/src/main/scala/kafka/admin/ListTopicCommand.scala c760cc0 
>   core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala d5de5f3 
>   core/src/main/scala/kafka/admin/ReassignPartitionsCommand.scala aa61fa1 
>   core/src/main/scala/kafka/client/ClientUtils.scala cc526ec 
>   core/src/main/scala/kafka/cluster/Broker.scala b03dea2 
>   core/src/main/scala/kafka/consumer/ConsoleConsumer.scala 719beb5 
>   core/src/main/scala/kafka/consumer/ConsumerFetcherManager.scala fa6b213 
>   core/src/main/scala/kafka/consumer/SimpleConsumer.scala 4395fe3 
>   core/src/main/scala/kafka/consumer/TopicCount.scala c8e8406 
>   core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala e7a692a 
>   core/src/main/scala/kafka/consumer/ZookeeperTopicEventWatcher.scala df83baa 
>   core/src/main/scala/kafka/controller/ControllerChannelManager.scala ed1ce0b 
>   core/src/main/scala/kafka/controller/KafkaController.scala ab18b7a 
>   core/src/main/scala/kafka/controller/PartitionStateMachine.scala a084830 
>   core/src/main/scala/kafka/controller/ReplicaStateMachine.scala c964857 
>   core/src/main/scala/kafka/javaapi/FetchRequest.scala b475240 
>   core/src/main/scala/kafka/javaapi/Implicits.scala ee0a71d 
>   core/src/main/scala/kafka/javaapi/OffsetRequest.scala 1c77ff8 
>   core/src/main/scala/kafka/javaapi/TopicMetadata.scala 97b6dcd 
>   core/src/main/scala/kafka/javaapi/TopicMetadataRequest.scala 5f80df7 
>   core/src/main/scala/kafka/javaapi/consumer/ZookeeperConsumerConnector.scala 14c4c8a 
>   core/src/main/scala/kafka/javaapi/message/ByteBufferMessageSet.scala 0a95248 
>   core/src/main/scala/kafka/javaapi/producer/Producer.scala 7265328 
>   core/src/main/scala/kafka/log/LogManager.scala 4771d11 
>   core/src/main/scala/kafka/network/BoundedByteBufferReceive.scala cab1864 
>   core/src/main/scala/kafka/producer/SyncProducer.scala 306f200 
>   core/src/main/scala/kafka/producer/async/DefaultEventHandler.scala 2e36d3b 
>   core/src/main/scala/kafka/producer/async/ProducerSendThread.scala 2b41a49 
>   core/src/main/scala/kafka/server/AbstractFetcherThread.scala d5addb3 
>   core/src/main/scala/kafka/server/KafkaApis.scala cd02aab 
>   core/src/main/scala/kafka/server/KafkaServerStartable.scala 5be65e9 
>   core/src/main/scala/kafka/server/ReplicaManager.scala f551243 
>   core/src/main/scala/kafka/server/ZookeeperLeaderElector.scala f1f0625 
>   core/src/main/scala/kafka/tools/ImportZkOffsets.scala 55709b5 
>   core/src/main/scala/kafka/tools/JmxTool.scala 7e424e7 
>   core/src/main/scala/kafka/tools/MirrorMaker.scala 6fb545a 
>   core/src/main/scala/kafka/tools/SimpleConsumerShell.scala 3cfa384 
>   core/src/main/scala/kafka/utils/Annotations.scala 28269eb 
>   core/src/main/scala/kafka/utils/Annotations_2.8.scala PRE-CREATION 
>   core/src/main/scala/kafka/utils/Annotations_2.9+.scala PRE-CREATION 
>   core/src/main/scala/kafka/utils/Json.scala f80b2cc 
>   core/src/main/scala/kafka/utils/Mx4jLoader.scala 64d84cc 
>   core/src/main/scala/kafka/utils/Pool.scala 9a86eab 
>   core/src/main/scala/kafka/utils/Utils.scala e83eb5f 
>   core/src/main/scala/kafka/utils/ZkUtils.scala ca1ce12 
>   core/src/test/scala/unit/kafka/admin/AddPartitionsTest.scala 06be990 
>   core/src/test/scala/unit/kafka/admin/AdminTest.scala dc0013f 
>   core/src/test/scala/unit/kafka/consumer/ZookeeperConsumerConnectorTest.scala fcfc583 
>   core/src/test/scala/unit/kafka/javaapi/consumer/ZookeeperConsumerConnectorTest.scala 9f243f0 
>   core/src/test/scala/unit/kafka/javaapi/message/BaseMessageSetTestCases.scala abee11b 
>   core/src/test/scala/unit/kafka/log/LogTest.scala 4ed88e8 
>   core/src/test/scala/unit/kafka/metrics/KafkaTimerTest.scala fe5bc09 
>   core/src/test/scala/unit/kafka/producer/AsyncProducerTest.scala 1781bc0 
>   core/src/test/scala/unit/kafka/producer/ProducerTest.scala 29331db 
>   core/src/test/scala/unit/kafka/producer/SyncProducerTest.scala b3e89c3 
>   core/src/test/scala/unit/kafka/utils/TestUtils.scala 830608f 
>   perf/src/main/scala/kafka/perf/ConsumerPerformance.scala 3158a22 
>   project/Build.scala b3858f3 
> 
> Diff: https://reviews.apache.org/r/14013/diff/
> 
> 
> Testing
> -------
> 
> successfully performed cross build across all scala versions
> unit tests pass across all scala versions
> 
> 
> Thanks,
> 
> Christopher Freeman
> 
>


Re: Review Request 14013: Added support for Scala 2.10 builds while maintaining compatibility with 2.8.x

Posted by Jun Rao <ju...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14013/#review25979
-----------------------------------------------------------


I am wondering if there is any performance degradation due to the changes in implicit conversion. Could you run kafka.perf.ProducerPerformance and see if there is any noticeable performance change with and w/o the patch?


core/src/main/scala/kafka/controller/PartitionStateMachine.scala
<https://reviews.apache.org/r/14013/#comment50705>

    Could you move the import of the JavaConversion to within this function to limit the scope of the implicit conversion?



core/src/main/scala/kafka/javaapi/consumer/ZookeeperConsumerConnector.scala
<https://reviews.apache.org/r/14013/#comment50706>

    Don't you need to specify the java return type explicitly here?



core/src/main/scala/kafka/utils/Annotations_2.9+.scala
<https://reviews.apache.org/r/14013/#comment50707>

    How does this affect IDEs like Intellij? Are you able to build the project in IDE with both version of threadsafe annotation?


- Jun Rao


On Sept. 8, 2013, 9:47 p.m., Christopher Freeman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14013/
> -----------------------------------------------------------
> 
> (Updated Sept. 8, 2013, 9:47 p.m.)
> 
> 
> Review request for kafka and Neha Narkhede.
> 
> 
> Bugs: KAFKA-1046
>     https://issues.apache.org/jira/browse/KAFKA-1046
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> This RB contains necessary changes to the kafka source code to support Scala 2.10.x while still maintaining support for Scala 2.8.x
> 
> The existing cause of source incompatibility between 2.8.x and 2.10.x was primarily due to the use of implicit conversions in the scala.collection.JavaConversions object.  Many of them where deprecated (and replaced with differently named methods) and the deprecated methods where removed in 2.10.  I avoided the source incompatibility by importing the implicits with a wild card.
> 
> Scala annotations moved to a different package in 2.9 and the reference to the old location was removed in 2.10.  I couldn't think of a source compatible way to support both in Annotations.scala so since this source file is very small, I created two copies and configured sbt to use the one appropriate for when building for a particular Scala version.
> 
> I also typed many of the catch block case statements with :Throwable.  This change isn't required but starting with Scala 2.9, the compiler will emit warnings if the type is emitted.  Otherwise "case e =>" and case "e: Throwable =>" are equivalent.
> 
> 
>   
> 
> 
> Diffs
> -----
> 
>   core/build.sbt c54cf44 
>   core/src/main/scala/kafka/Kafka.scala dafb1ee 
>   core/src/main/scala/kafka/admin/AddPartitionsCommand.scala 5757c32 
>   core/src/main/scala/kafka/admin/AdminUtils.scala c399bc7 
>   core/src/main/scala/kafka/admin/CreateTopicCommand.scala 21c1186 
>   core/src/main/scala/kafka/admin/DeleteTopicCommand.scala 3da4518 
>   core/src/main/scala/kafka/admin/ListTopicCommand.scala c760cc0 
>   core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala d5de5f3 
>   core/src/main/scala/kafka/admin/ReassignPartitionsCommand.scala aa61fa1 
>   core/src/main/scala/kafka/client/ClientUtils.scala cc526ec 
>   core/src/main/scala/kafka/cluster/Broker.scala b03dea2 
>   core/src/main/scala/kafka/consumer/ConsoleConsumer.scala 719beb5 
>   core/src/main/scala/kafka/consumer/ConsumerFetcherManager.scala fa6b213 
>   core/src/main/scala/kafka/consumer/SimpleConsumer.scala 4395fe3 
>   core/src/main/scala/kafka/consumer/TopicCount.scala c8e8406 
>   core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala e7a692a 
>   core/src/main/scala/kafka/consumer/ZookeeperTopicEventWatcher.scala df83baa 
>   core/src/main/scala/kafka/controller/ControllerChannelManager.scala ed1ce0b 
>   core/src/main/scala/kafka/controller/KafkaController.scala ab18b7a 
>   core/src/main/scala/kafka/controller/PartitionStateMachine.scala a084830 
>   core/src/main/scala/kafka/controller/ReplicaStateMachine.scala c964857 
>   core/src/main/scala/kafka/javaapi/FetchRequest.scala b475240 
>   core/src/main/scala/kafka/javaapi/Implicits.scala ee0a71d 
>   core/src/main/scala/kafka/javaapi/OffsetRequest.scala 1c77ff8 
>   core/src/main/scala/kafka/javaapi/TopicMetadata.scala 97b6dcd 
>   core/src/main/scala/kafka/javaapi/TopicMetadataRequest.scala 5f80df7 
>   core/src/main/scala/kafka/javaapi/consumer/ZookeeperConsumerConnector.scala 14c4c8a 
>   core/src/main/scala/kafka/javaapi/message/ByteBufferMessageSet.scala 0a95248 
>   core/src/main/scala/kafka/javaapi/producer/Producer.scala 7265328 
>   core/src/main/scala/kafka/log/LogManager.scala 4771d11 
>   core/src/main/scala/kafka/network/BoundedByteBufferReceive.scala cab1864 
>   core/src/main/scala/kafka/producer/SyncProducer.scala 306f200 
>   core/src/main/scala/kafka/producer/async/DefaultEventHandler.scala 2e36d3b 
>   core/src/main/scala/kafka/producer/async/ProducerSendThread.scala 2b41a49 
>   core/src/main/scala/kafka/server/AbstractFetcherThread.scala d5addb3 
>   core/src/main/scala/kafka/server/KafkaApis.scala cd02aab 
>   core/src/main/scala/kafka/server/KafkaServerStartable.scala 5be65e9 
>   core/src/main/scala/kafka/server/ReplicaManager.scala f551243 
>   core/src/main/scala/kafka/server/ZookeeperLeaderElector.scala f1f0625 
>   core/src/main/scala/kafka/tools/ImportZkOffsets.scala 55709b5 
>   core/src/main/scala/kafka/tools/JmxTool.scala 7e424e7 
>   core/src/main/scala/kafka/tools/MirrorMaker.scala 6fb545a 
>   core/src/main/scala/kafka/tools/SimpleConsumerShell.scala 3cfa384 
>   core/src/main/scala/kafka/utils/Annotations.scala 28269eb 
>   core/src/main/scala/kafka/utils/Annotations_2.8.scala PRE-CREATION 
>   core/src/main/scala/kafka/utils/Annotations_2.9+.scala PRE-CREATION 
>   core/src/main/scala/kafka/utils/Json.scala f80b2cc 
>   core/src/main/scala/kafka/utils/Mx4jLoader.scala 64d84cc 
>   core/src/main/scala/kafka/utils/Pool.scala 9a86eab 
>   core/src/main/scala/kafka/utils/Utils.scala e83eb5f 
>   core/src/main/scala/kafka/utils/ZkUtils.scala ca1ce12 
>   core/src/test/scala/unit/kafka/admin/AddPartitionsTest.scala 06be990 
>   core/src/test/scala/unit/kafka/admin/AdminTest.scala dc0013f 
>   core/src/test/scala/unit/kafka/consumer/ZookeeperConsumerConnectorTest.scala fcfc583 
>   core/src/test/scala/unit/kafka/javaapi/consumer/ZookeeperConsumerConnectorTest.scala 9f243f0 
>   core/src/test/scala/unit/kafka/javaapi/message/BaseMessageSetTestCases.scala abee11b 
>   core/src/test/scala/unit/kafka/log/LogTest.scala 4ed88e8 
>   core/src/test/scala/unit/kafka/metrics/KafkaTimerTest.scala fe5bc09 
>   core/src/test/scala/unit/kafka/producer/AsyncProducerTest.scala 1781bc0 
>   core/src/test/scala/unit/kafka/producer/ProducerTest.scala 29331db 
>   core/src/test/scala/unit/kafka/producer/SyncProducerTest.scala b3e89c3 
>   core/src/test/scala/unit/kafka/utils/TestUtils.scala 830608f 
>   perf/src/main/scala/kafka/perf/ConsumerPerformance.scala 3158a22 
>   project/Build.scala b3858f3 
> 
> Diff: https://reviews.apache.org/r/14013/diff/
> 
> 
> Testing
> -------
> 
> successfully performed cross build across all scala versions
> unit tests pass across all scala versions
> 
> 
> Thanks,
> 
> Christopher Freeman
> 
>


Re: Review Request 14013: Added support for Scala 2.10 builds while maintaining compatibility with 2.8.x

Posted by Neha Narkhede <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14013/#review25992
-----------------------------------------------------------

Ship it!


Ship It!

- Neha Narkhede


On Sept. 9, 2013, 12:40 a.m., Christopher Freeman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14013/
> -----------------------------------------------------------
> 
> (Updated Sept. 9, 2013, 12:40 a.m.)
> 
> 
> Review request for kafka and Neha Narkhede.
> 
> 
> Bugs: KAFKA-1046
>     https://issues.apache.org/jira/browse/KAFKA-1046
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> This RB contains necessary changes to the kafka source code to support Scala 2.10.x while still maintaining support for Scala 2.8.x
> 
> The existing cause of source incompatibility between 2.8.x and 2.10.x was primarily due to the use of implicit conversions in the scala.collection.JavaConversions object.  Many of them where deprecated (and replaced with differently named methods) and the deprecated methods where removed in 2.10.  I avoided the source incompatibility by importing the implicits with a wild card.
> 
> Scala annotations moved to a different package in 2.9 and the reference to the old location was removed in 2.10.  I couldn't think of a source compatible way to support both in Annotations.scala so since this source file is very small, I created two copies and configured sbt to use the one appropriate for when building for a particular Scala version.
> 
> I also typed many of the catch block case statements with :Throwable.  This change isn't required but starting with Scala 2.9, the compiler will emit warnings if the type is emitted.  Otherwise "case e =>" and case "e: Throwable =>" are equivalent.
> 
> 
>   
> 
> 
> Diffs
> -----
> 
>   core/build.sbt c54cf44 
>   core/src/main/scala/kafka/Kafka.scala dafb1ee 
>   core/src/main/scala/kafka/admin/AddPartitionsCommand.scala 5757c32 
>   core/src/main/scala/kafka/admin/AdminUtils.scala c399bc7 
>   core/src/main/scala/kafka/admin/CreateTopicCommand.scala 21c1186 
>   core/src/main/scala/kafka/admin/DeleteTopicCommand.scala 3da4518 
>   core/src/main/scala/kafka/admin/ListTopicCommand.scala c760cc0 
>   core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala d5de5f3 
>   core/src/main/scala/kafka/admin/ReassignPartitionsCommand.scala aa61fa1 
>   core/src/main/scala/kafka/client/ClientUtils.scala cc526ec 
>   core/src/main/scala/kafka/cluster/Broker.scala b03dea2 
>   core/src/main/scala/kafka/consumer/ConsoleConsumer.scala 719beb5 
>   core/src/main/scala/kafka/consumer/ConsumerFetcherManager.scala fa6b213 
>   core/src/main/scala/kafka/consumer/SimpleConsumer.scala 4395fe3 
>   core/src/main/scala/kafka/consumer/TopicCount.scala c8e8406 
>   core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala e7a692a 
>   core/src/main/scala/kafka/consumer/ZookeeperTopicEventWatcher.scala df83baa 
>   core/src/main/scala/kafka/controller/ControllerChannelManager.scala ed1ce0b 
>   core/src/main/scala/kafka/controller/KafkaController.scala ab18b7a 
>   core/src/main/scala/kafka/controller/PartitionStateMachine.scala a084830 
>   core/src/main/scala/kafka/controller/ReplicaStateMachine.scala c964857 
>   core/src/main/scala/kafka/javaapi/FetchRequest.scala b475240 
>   core/src/main/scala/kafka/javaapi/Implicits.scala ee0a71d 
>   core/src/main/scala/kafka/javaapi/OffsetRequest.scala 1c77ff8 
>   core/src/main/scala/kafka/javaapi/TopicMetadata.scala 97b6dcd 
>   core/src/main/scala/kafka/javaapi/TopicMetadataRequest.scala 5f80df7 
>   core/src/main/scala/kafka/javaapi/consumer/ZookeeperConsumerConnector.scala 14c4c8a 
>   core/src/main/scala/kafka/javaapi/message/ByteBufferMessageSet.scala 0a95248 
>   core/src/main/scala/kafka/javaapi/producer/Producer.scala 7265328 
>   core/src/main/scala/kafka/log/LogManager.scala 4771d11 
>   core/src/main/scala/kafka/network/BoundedByteBufferReceive.scala cab1864 
>   core/src/main/scala/kafka/producer/SyncProducer.scala 306f200 
>   core/src/main/scala/kafka/producer/async/DefaultEventHandler.scala 2e36d3b 
>   core/src/main/scala/kafka/producer/async/ProducerSendThread.scala 2b41a49 
>   core/src/main/scala/kafka/server/AbstractFetcherThread.scala d5addb3 
>   core/src/main/scala/kafka/server/KafkaApis.scala cd02aab 
>   core/src/main/scala/kafka/server/KafkaServerStartable.scala 5be65e9 
>   core/src/main/scala/kafka/server/ReplicaManager.scala f551243 
>   core/src/main/scala/kafka/server/ZookeeperLeaderElector.scala f1f0625 
>   core/src/main/scala/kafka/tools/ImportZkOffsets.scala 55709b5 
>   core/src/main/scala/kafka/tools/JmxTool.scala 7e424e7 
>   core/src/main/scala/kafka/tools/MirrorMaker.scala 6fb545a 
>   core/src/main/scala/kafka/tools/SimpleConsumerShell.scala 3cfa384 
>   core/src/main/scala/kafka/utils/Annotations.scala 28269eb 
>   core/src/main/scala/kafka/utils/Annotations_2.8.scala PRE-CREATION 
>   core/src/main/scala/kafka/utils/Annotations_2.9+.scala PRE-CREATION 
>   core/src/main/scala/kafka/utils/Json.scala f80b2cc 
>   core/src/main/scala/kafka/utils/Mx4jLoader.scala 64d84cc 
>   core/src/main/scala/kafka/utils/Pool.scala 9a86eab 
>   core/src/main/scala/kafka/utils/Utils.scala e83eb5f 
>   core/src/main/scala/kafka/utils/ZkUtils.scala ca1ce12 
>   core/src/test/scala/unit/kafka/admin/AddPartitionsTest.scala 06be990 
>   core/src/test/scala/unit/kafka/admin/AdminTest.scala dc0013f 
>   core/src/test/scala/unit/kafka/consumer/ZookeeperConsumerConnectorTest.scala fcfc583 
>   core/src/test/scala/unit/kafka/javaapi/consumer/ZookeeperConsumerConnectorTest.scala 9f243f0 
>   core/src/test/scala/unit/kafka/javaapi/message/BaseMessageSetTestCases.scala abee11b 
>   core/src/test/scala/unit/kafka/log/LogTest.scala 4ed88e8 
>   core/src/test/scala/unit/kafka/metrics/KafkaTimerTest.scala fe5bc09 
>   core/src/test/scala/unit/kafka/producer/AsyncProducerTest.scala 1781bc0 
>   core/src/test/scala/unit/kafka/producer/ProducerTest.scala 29331db 
>   core/src/test/scala/unit/kafka/producer/SyncProducerTest.scala b3e89c3 
>   core/src/test/scala/unit/kafka/utils/TestUtils.scala 830608f 
>   perf/src/main/scala/kafka/perf/ConsumerPerformance.scala 3158a22 
>   project/Build.scala b3858f3 
> 
> Diff: https://reviews.apache.org/r/14013/diff/
> 
> 
> Testing
> -------
> 
> successfully performed cross build across all scala versions
> unit tests pass across all scala versions
> 
> 
> Thanks,
> 
> Christopher Freeman
> 
>


Re: Review Request 14013: Added support for Scala 2.10 builds while maintaining compatibility with 2.8.x

Posted by Christopher Freeman <cf...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14013/
-----------------------------------------------------------

(Updated Sept. 9, 2013, 12:40 a.m.)


Review request for kafka and Neha Narkhede.


Changes
-------

addressed Jun's feedback


Bugs: KAFKA-1046
    https://issues.apache.org/jira/browse/KAFKA-1046


Repository: kafka


Description
-------

This RB contains necessary changes to the kafka source code to support Scala 2.10.x while still maintaining support for Scala 2.8.x

The existing cause of source incompatibility between 2.8.x and 2.10.x was primarily due to the use of implicit conversions in the scala.collection.JavaConversions object.  Many of them where deprecated (and replaced with differently named methods) and the deprecated methods where removed in 2.10.  I avoided the source incompatibility by importing the implicits with a wild card.

Scala annotations moved to a different package in 2.9 and the reference to the old location was removed in 2.10.  I couldn't think of a source compatible way to support both in Annotations.scala so since this source file is very small, I created two copies and configured sbt to use the one appropriate for when building for a particular Scala version.

I also typed many of the catch block case statements with :Throwable.  This change isn't required but starting with Scala 2.9, the compiler will emit warnings if the type is emitted.  Otherwise "case e =>" and case "e: Throwable =>" are equivalent.


  


Diffs (updated)
-----

  core/build.sbt c54cf44 
  core/src/main/scala/kafka/Kafka.scala dafb1ee 
  core/src/main/scala/kafka/admin/AddPartitionsCommand.scala 5757c32 
  core/src/main/scala/kafka/admin/AdminUtils.scala c399bc7 
  core/src/main/scala/kafka/admin/CreateTopicCommand.scala 21c1186 
  core/src/main/scala/kafka/admin/DeleteTopicCommand.scala 3da4518 
  core/src/main/scala/kafka/admin/ListTopicCommand.scala c760cc0 
  core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala d5de5f3 
  core/src/main/scala/kafka/admin/ReassignPartitionsCommand.scala aa61fa1 
  core/src/main/scala/kafka/client/ClientUtils.scala cc526ec 
  core/src/main/scala/kafka/cluster/Broker.scala b03dea2 
  core/src/main/scala/kafka/consumer/ConsoleConsumer.scala 719beb5 
  core/src/main/scala/kafka/consumer/ConsumerFetcherManager.scala fa6b213 
  core/src/main/scala/kafka/consumer/SimpleConsumer.scala 4395fe3 
  core/src/main/scala/kafka/consumer/TopicCount.scala c8e8406 
  core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala e7a692a 
  core/src/main/scala/kafka/consumer/ZookeeperTopicEventWatcher.scala df83baa 
  core/src/main/scala/kafka/controller/ControllerChannelManager.scala ed1ce0b 
  core/src/main/scala/kafka/controller/KafkaController.scala ab18b7a 
  core/src/main/scala/kafka/controller/PartitionStateMachine.scala a084830 
  core/src/main/scala/kafka/controller/ReplicaStateMachine.scala c964857 
  core/src/main/scala/kafka/javaapi/FetchRequest.scala b475240 
  core/src/main/scala/kafka/javaapi/Implicits.scala ee0a71d 
  core/src/main/scala/kafka/javaapi/OffsetRequest.scala 1c77ff8 
  core/src/main/scala/kafka/javaapi/TopicMetadata.scala 97b6dcd 
  core/src/main/scala/kafka/javaapi/TopicMetadataRequest.scala 5f80df7 
  core/src/main/scala/kafka/javaapi/consumer/ZookeeperConsumerConnector.scala 14c4c8a 
  core/src/main/scala/kafka/javaapi/message/ByteBufferMessageSet.scala 0a95248 
  core/src/main/scala/kafka/javaapi/producer/Producer.scala 7265328 
  core/src/main/scala/kafka/log/LogManager.scala 4771d11 
  core/src/main/scala/kafka/network/BoundedByteBufferReceive.scala cab1864 
  core/src/main/scala/kafka/producer/SyncProducer.scala 306f200 
  core/src/main/scala/kafka/producer/async/DefaultEventHandler.scala 2e36d3b 
  core/src/main/scala/kafka/producer/async/ProducerSendThread.scala 2b41a49 
  core/src/main/scala/kafka/server/AbstractFetcherThread.scala d5addb3 
  core/src/main/scala/kafka/server/KafkaApis.scala cd02aab 
  core/src/main/scala/kafka/server/KafkaServerStartable.scala 5be65e9 
  core/src/main/scala/kafka/server/ReplicaManager.scala f551243 
  core/src/main/scala/kafka/server/ZookeeperLeaderElector.scala f1f0625 
  core/src/main/scala/kafka/tools/ImportZkOffsets.scala 55709b5 
  core/src/main/scala/kafka/tools/JmxTool.scala 7e424e7 
  core/src/main/scala/kafka/tools/MirrorMaker.scala 6fb545a 
  core/src/main/scala/kafka/tools/SimpleConsumerShell.scala 3cfa384 
  core/src/main/scala/kafka/utils/Annotations.scala 28269eb 
  core/src/main/scala/kafka/utils/Annotations_2.8.scala PRE-CREATION 
  core/src/main/scala/kafka/utils/Annotations_2.9+.scala PRE-CREATION 
  core/src/main/scala/kafka/utils/Json.scala f80b2cc 
  core/src/main/scala/kafka/utils/Mx4jLoader.scala 64d84cc 
  core/src/main/scala/kafka/utils/Pool.scala 9a86eab 
  core/src/main/scala/kafka/utils/Utils.scala e83eb5f 
  core/src/main/scala/kafka/utils/ZkUtils.scala ca1ce12 
  core/src/test/scala/unit/kafka/admin/AddPartitionsTest.scala 06be990 
  core/src/test/scala/unit/kafka/admin/AdminTest.scala dc0013f 
  core/src/test/scala/unit/kafka/consumer/ZookeeperConsumerConnectorTest.scala fcfc583 
  core/src/test/scala/unit/kafka/javaapi/consumer/ZookeeperConsumerConnectorTest.scala 9f243f0 
  core/src/test/scala/unit/kafka/javaapi/message/BaseMessageSetTestCases.scala abee11b 
  core/src/test/scala/unit/kafka/log/LogTest.scala 4ed88e8 
  core/src/test/scala/unit/kafka/metrics/KafkaTimerTest.scala fe5bc09 
  core/src/test/scala/unit/kafka/producer/AsyncProducerTest.scala 1781bc0 
  core/src/test/scala/unit/kafka/producer/ProducerTest.scala 29331db 
  core/src/test/scala/unit/kafka/producer/SyncProducerTest.scala b3e89c3 
  core/src/test/scala/unit/kafka/utils/TestUtils.scala 830608f 
  perf/src/main/scala/kafka/perf/ConsumerPerformance.scala 3158a22 
  project/Build.scala b3858f3 

Diff: https://reviews.apache.org/r/14013/diff/


Testing
-------

successfully performed cross build across all scala versions
unit tests pass across all scala versions


Thanks,

Christopher Freeman


Re: Review Request 14013: Added support for Scala 2.10 builds while maintaining compatibility with 2.8.x

Posted by Christopher Freeman <cf...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14013/
-----------------------------------------------------------

(Updated Sept. 8, 2013, 9:47 p.m.)


Review request for kafka and Neha Narkhede.


Changes
-------

addressed feedback and updated the diff


Bugs: KAFKA-1046
    https://issues.apache.org/jira/browse/KAFKA-1046


Repository: kafka


Description
-------

This RB contains necessary changes to the kafka source code to support Scala 2.10.x while still maintaining support for Scala 2.8.x

The existing cause of source incompatibility between 2.8.x and 2.10.x was primarily due to the use of implicit conversions in the scala.collection.JavaConversions object.  Many of them where deprecated (and replaced with differently named methods) and the deprecated methods where removed in 2.10.  I avoided the source incompatibility by importing the implicits with a wild card.

Scala annotations moved to a different package in 2.9 and the reference to the old location was removed in 2.10.  I couldn't think of a source compatible way to support both in Annotations.scala so since this source file is very small, I created two copies and configured sbt to use the one appropriate for when building for a particular Scala version.

I also typed many of the catch block case statements with :Throwable.  This change isn't required but starting with Scala 2.9, the compiler will emit warnings if the type is emitted.  Otherwise "case e =>" and case "e: Throwable =>" are equivalent.


  


Diffs (updated)
-----

  core/build.sbt c54cf44 
  core/src/main/scala/kafka/Kafka.scala dafb1ee 
  core/src/main/scala/kafka/admin/AddPartitionsCommand.scala 5757c32 
  core/src/main/scala/kafka/admin/AdminUtils.scala c399bc7 
  core/src/main/scala/kafka/admin/CreateTopicCommand.scala 21c1186 
  core/src/main/scala/kafka/admin/DeleteTopicCommand.scala 3da4518 
  core/src/main/scala/kafka/admin/ListTopicCommand.scala c760cc0 
  core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala d5de5f3 
  core/src/main/scala/kafka/admin/ReassignPartitionsCommand.scala aa61fa1 
  core/src/main/scala/kafka/client/ClientUtils.scala cc526ec 
  core/src/main/scala/kafka/cluster/Broker.scala b03dea2 
  core/src/main/scala/kafka/consumer/ConsoleConsumer.scala 719beb5 
  core/src/main/scala/kafka/consumer/ConsumerFetcherManager.scala fa6b213 
  core/src/main/scala/kafka/consumer/SimpleConsumer.scala 4395fe3 
  core/src/main/scala/kafka/consumer/TopicCount.scala c8e8406 
  core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala e7a692a 
  core/src/main/scala/kafka/consumer/ZookeeperTopicEventWatcher.scala df83baa 
  core/src/main/scala/kafka/controller/ControllerChannelManager.scala ed1ce0b 
  core/src/main/scala/kafka/controller/KafkaController.scala ab18b7a 
  core/src/main/scala/kafka/controller/PartitionStateMachine.scala a084830 
  core/src/main/scala/kafka/controller/ReplicaStateMachine.scala c964857 
  core/src/main/scala/kafka/javaapi/FetchRequest.scala b475240 
  core/src/main/scala/kafka/javaapi/Implicits.scala ee0a71d 
  core/src/main/scala/kafka/javaapi/OffsetRequest.scala 1c77ff8 
  core/src/main/scala/kafka/javaapi/TopicMetadata.scala 97b6dcd 
  core/src/main/scala/kafka/javaapi/TopicMetadataRequest.scala 5f80df7 
  core/src/main/scala/kafka/javaapi/consumer/ZookeeperConsumerConnector.scala 14c4c8a 
  core/src/main/scala/kafka/javaapi/message/ByteBufferMessageSet.scala 0a95248 
  core/src/main/scala/kafka/javaapi/producer/Producer.scala 7265328 
  core/src/main/scala/kafka/log/LogManager.scala 4771d11 
  core/src/main/scala/kafka/network/BoundedByteBufferReceive.scala cab1864 
  core/src/main/scala/kafka/producer/SyncProducer.scala 306f200 
  core/src/main/scala/kafka/producer/async/DefaultEventHandler.scala 2e36d3b 
  core/src/main/scala/kafka/producer/async/ProducerSendThread.scala 2b41a49 
  core/src/main/scala/kafka/server/AbstractFetcherThread.scala d5addb3 
  core/src/main/scala/kafka/server/KafkaApis.scala cd02aab 
  core/src/main/scala/kafka/server/KafkaServerStartable.scala 5be65e9 
  core/src/main/scala/kafka/server/ReplicaManager.scala f551243 
  core/src/main/scala/kafka/server/ZookeeperLeaderElector.scala f1f0625 
  core/src/main/scala/kafka/tools/ImportZkOffsets.scala 55709b5 
  core/src/main/scala/kafka/tools/JmxTool.scala 7e424e7 
  core/src/main/scala/kafka/tools/MirrorMaker.scala 6fb545a 
  core/src/main/scala/kafka/tools/SimpleConsumerShell.scala 3cfa384 
  core/src/main/scala/kafka/utils/Annotations.scala 28269eb 
  core/src/main/scala/kafka/utils/Annotations_2.8.scala PRE-CREATION 
  core/src/main/scala/kafka/utils/Annotations_2.9+.scala PRE-CREATION 
  core/src/main/scala/kafka/utils/Json.scala f80b2cc 
  core/src/main/scala/kafka/utils/Mx4jLoader.scala 64d84cc 
  core/src/main/scala/kafka/utils/Pool.scala 9a86eab 
  core/src/main/scala/kafka/utils/Utils.scala e83eb5f 
  core/src/main/scala/kafka/utils/ZkUtils.scala ca1ce12 
  core/src/test/scala/unit/kafka/admin/AddPartitionsTest.scala 06be990 
  core/src/test/scala/unit/kafka/admin/AdminTest.scala dc0013f 
  core/src/test/scala/unit/kafka/consumer/ZookeeperConsumerConnectorTest.scala fcfc583 
  core/src/test/scala/unit/kafka/javaapi/consumer/ZookeeperConsumerConnectorTest.scala 9f243f0 
  core/src/test/scala/unit/kafka/javaapi/message/BaseMessageSetTestCases.scala abee11b 
  core/src/test/scala/unit/kafka/log/LogTest.scala 4ed88e8 
  core/src/test/scala/unit/kafka/metrics/KafkaTimerTest.scala fe5bc09 
  core/src/test/scala/unit/kafka/producer/AsyncProducerTest.scala 1781bc0 
  core/src/test/scala/unit/kafka/producer/ProducerTest.scala 29331db 
  core/src/test/scala/unit/kafka/producer/SyncProducerTest.scala b3e89c3 
  core/src/test/scala/unit/kafka/utils/TestUtils.scala 830608f 
  perf/src/main/scala/kafka/perf/ConsumerPerformance.scala 3158a22 
  project/Build.scala b3858f3 

Diff: https://reviews.apache.org/r/14013/diff/


Testing
-------

successfully performed cross build across all scala versions
unit tests pass across all scala versions


Thanks,

Christopher Freeman


Re: Review Request 14013: Added support for Scala 2.10 builds while maintaining compatibility with 2.8.x

Posted by Christopher Freeman <cf...@linkedin.com>.

> On Sept. 8, 2013, 4:56 p.m., Neha Narkhede wrote:
> > core/build.sbt, line 26
> > <https://reviews.apache.org/r/14013/diff/2/?file=349260#file349260line26>
> >
> >     Does Scala 2.10 require scalatest 1.9.1? 
> >     How about Scala 2.9.x ?

There wasn't a 2.10 build of scalatest version 1.8 so I needed to use one of the newer versions with a 2.10 build.  For the other cross build cases, they will continue to work with the same version of scalatest that they did previously.


> On Sept. 8, 2013, 4:56 p.m., Neha Narkhede wrote:
> > core/src/main/scala/kafka/javaapi/Implicits.scala, line 44
> > <https://reviews.apache.org/r/14013/diff/2/?file=349282#file349282line44>
> >
> >     Could you please add a comment above this API explaining why we need to do this ? The same one you have in ByteBufferMessageSet.scala

sure


> On Sept. 8, 2013, 4:56 p.m., Neha Narkhede wrote:
> > core/src/main/scala/kafka/utils/Pool.scala, line 79
> > <https://reviews.apache.org/r/14013/diff/2/?file=349308#file349308line79>
> >
> >     In all other places, we have limited the scope of the JavaConversions import. Could we also do the same here?

yes


- Christopher


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14013/#review25977
-----------------------------------------------------------


On Sept. 7, 2013, 11:35 p.m., Christopher Freeman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14013/
> -----------------------------------------------------------
> 
> (Updated Sept. 7, 2013, 11:35 p.m.)
> 
> 
> Review request for kafka and Neha Narkhede.
> 
> 
> Bugs: KAFKA-1046
>     https://issues.apache.org/jira/browse/KAFKA-1046
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> This RB contains necessary changes to the kafka source code to support Scala 2.10.x while still maintaining support for Scala 2.8.x
> 
> The existing cause of source incompatibility between 2.8.x and 2.10.x was primarily due to the use of implicit conversions in the scala.collection.JavaConversions object.  Many of them where deprecated (and replaced with differently named methods) and the deprecated methods where removed in 2.10.  I avoided the source incompatibility by importing the implicits with a wild card.
> 
> Scala annotations moved to a different package in 2.9 and the reference to the old location was removed in 2.10.  I couldn't think of a source compatible way to support both in Annotations.scala so since this source file is very small, I created two copies and configured sbt to use the one appropriate for when building for a particular Scala version.
> 
> I also typed many of the catch block case statements with :Throwable.  This change isn't required but starting with Scala 2.9, the compiler will emit warnings if the type is emitted.  Otherwise "case e =>" and case "e: Throwable =>" are equivalent.
> 
> 
>   
> 
> 
> Diffs
> -----
> 
>   core/build.sbt c54cf44 
>   core/src/main/scala/kafka/Kafka.scala dafb1ee 
>   core/src/main/scala/kafka/admin/AddPartitionsCommand.scala 5757c32 
>   core/src/main/scala/kafka/admin/AdminUtils.scala c399bc7 
>   core/src/main/scala/kafka/admin/CreateTopicCommand.scala 21c1186 
>   core/src/main/scala/kafka/admin/DeleteTopicCommand.scala 3da4518 
>   core/src/main/scala/kafka/admin/ListTopicCommand.scala c760cc0 
>   core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala d5de5f3 
>   core/src/main/scala/kafka/admin/ReassignPartitionsCommand.scala aa61fa1 
>   core/src/main/scala/kafka/client/ClientUtils.scala cc526ec 
>   core/src/main/scala/kafka/cluster/Broker.scala b03dea2 
>   core/src/main/scala/kafka/consumer/ConsoleConsumer.scala 719beb5 
>   core/src/main/scala/kafka/consumer/ConsumerFetcherManager.scala fa6b213 
>   core/src/main/scala/kafka/consumer/SimpleConsumer.scala 4395fe3 
>   core/src/main/scala/kafka/consumer/TopicCount.scala c8e8406 
>   core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala e7a692a 
>   core/src/main/scala/kafka/consumer/ZookeeperTopicEventWatcher.scala df83baa 
>   core/src/main/scala/kafka/controller/ControllerChannelManager.scala ed1ce0b 
>   core/src/main/scala/kafka/controller/KafkaController.scala ab18b7a 
>   core/src/main/scala/kafka/controller/PartitionStateMachine.scala a084830 
>   core/src/main/scala/kafka/controller/ReplicaStateMachine.scala c964857 
>   core/src/main/scala/kafka/javaapi/FetchRequest.scala b475240 
>   core/src/main/scala/kafka/javaapi/Implicits.scala ee0a71d 
>   core/src/main/scala/kafka/javaapi/OffsetRequest.scala 1c77ff8 
>   core/src/main/scala/kafka/javaapi/TopicMetadata.scala 97b6dcd 
>   core/src/main/scala/kafka/javaapi/TopicMetadataRequest.scala 5f80df7 
>   core/src/main/scala/kafka/javaapi/consumer/ZookeeperConsumerConnector.scala 14c4c8a 
>   core/src/main/scala/kafka/javaapi/message/ByteBufferMessageSet.scala 0a95248 
>   core/src/main/scala/kafka/javaapi/producer/Producer.scala 7265328 
>   core/src/main/scala/kafka/log/LogManager.scala 4771d11 
>   core/src/main/scala/kafka/network/BoundedByteBufferReceive.scala cab1864 
>   core/src/main/scala/kafka/producer/SyncProducer.scala 306f200 
>   core/src/main/scala/kafka/producer/async/DefaultEventHandler.scala 2e36d3b 
>   core/src/main/scala/kafka/producer/async/ProducerSendThread.scala 2b41a49 
>   core/src/main/scala/kafka/server/AbstractFetcherThread.scala d5addb3 
>   core/src/main/scala/kafka/server/KafkaApis.scala cd02aab 
>   core/src/main/scala/kafka/server/KafkaServerStartable.scala 5be65e9 
>   core/src/main/scala/kafka/server/ReplicaManager.scala f551243 
>   core/src/main/scala/kafka/server/ZookeeperLeaderElector.scala f1f0625 
>   core/src/main/scala/kafka/tools/ImportZkOffsets.scala 55709b5 
>   core/src/main/scala/kafka/tools/JmxTool.scala 7e424e7 
>   core/src/main/scala/kafka/tools/MirrorMaker.scala 6fb545a 
>   core/src/main/scala/kafka/tools/SimpleConsumerShell.scala 3cfa384 
>   core/src/main/scala/kafka/utils/Annotations.scala 28269eb 
>   core/src/main/scala/kafka/utils/Annotations_2.8.scala PRE-CREATION 
>   core/src/main/scala/kafka/utils/Annotations_2.9+.scala PRE-CREATION 
>   core/src/main/scala/kafka/utils/Json.scala f80b2cc 
>   core/src/main/scala/kafka/utils/Mx4jLoader.scala 64d84cc 
>   core/src/main/scala/kafka/utils/Pool.scala 9a86eab 
>   core/src/main/scala/kafka/utils/Utils.scala e83eb5f 
>   core/src/main/scala/kafka/utils/ZkUtils.scala ca1ce12 
>   core/src/test/scala/unit/kafka/admin/AddPartitionsTest.scala 06be990 
>   core/src/test/scala/unit/kafka/admin/AdminTest.scala dc0013f 
>   core/src/test/scala/unit/kafka/consumer/ZookeeperConsumerConnectorTest.scala fcfc583 
>   core/src/test/scala/unit/kafka/javaapi/consumer/ZookeeperConsumerConnectorTest.scala 9f243f0 
>   core/src/test/scala/unit/kafka/javaapi/message/BaseMessageSetTestCases.scala abee11b 
>   core/src/test/scala/unit/kafka/log/LogTest.scala 4ed88e8 
>   core/src/test/scala/unit/kafka/metrics/KafkaTimerTest.scala fe5bc09 
>   core/src/test/scala/unit/kafka/producer/AsyncProducerTest.scala 1781bc0 
>   core/src/test/scala/unit/kafka/producer/ProducerTest.scala 29331db 
>   core/src/test/scala/unit/kafka/producer/SyncProducerTest.scala b3e89c3 
>   core/src/test/scala/unit/kafka/utils/TestUtils.scala 830608f 
>   perf/src/main/scala/kafka/perf/ConsumerPerformance.scala 3158a22 
>   project/Build.scala b3858f3 
> 
> Diff: https://reviews.apache.org/r/14013/diff/
> 
> 
> Testing
> -------
> 
> successfully performed cross build across all scala versions
> unit tests pass across all scala versions
> 
> 
> Thanks,
> 
> Christopher Freeman
> 
>


Re: Review Request 14013: Added support for Scala 2.10 builds while maintaining compatibility with 2.8.x

Posted by Neha Narkhede <ne...@gmail.com>.

> On Sept. 8, 2013, 4:56 p.m., Neha Narkhede wrote:
> > core/build.sbt, line 26
> > <https://reviews.apache.org/r/14013/diff/2/?file=349260#file349260line26>
> >
> >     Does Scala 2.10 require scalatest 1.9.1? 
> >     How about Scala 2.9.x ?
> 
> Christopher Freeman wrote:
>     There wasn't a 2.10 build of scalatest version 1.8 so I needed to use one of the newer versions with a 2.10 build.  For the other cross build cases, they will continue to work with the same version of scalatest that they did previously.

Makes sense. Thanks!


- Neha


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14013/#review25977
-----------------------------------------------------------


On Sept. 9, 2013, 12:40 a.m., Christopher Freeman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14013/
> -----------------------------------------------------------
> 
> (Updated Sept. 9, 2013, 12:40 a.m.)
> 
> 
> Review request for kafka and Neha Narkhede.
> 
> 
> Bugs: KAFKA-1046
>     https://issues.apache.org/jira/browse/KAFKA-1046
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> This RB contains necessary changes to the kafka source code to support Scala 2.10.x while still maintaining support for Scala 2.8.x
> 
> The existing cause of source incompatibility between 2.8.x and 2.10.x was primarily due to the use of implicit conversions in the scala.collection.JavaConversions object.  Many of them where deprecated (and replaced with differently named methods) and the deprecated methods where removed in 2.10.  I avoided the source incompatibility by importing the implicits with a wild card.
> 
> Scala annotations moved to a different package in 2.9 and the reference to the old location was removed in 2.10.  I couldn't think of a source compatible way to support both in Annotations.scala so since this source file is very small, I created two copies and configured sbt to use the one appropriate for when building for a particular Scala version.
> 
> I also typed many of the catch block case statements with :Throwable.  This change isn't required but starting with Scala 2.9, the compiler will emit warnings if the type is emitted.  Otherwise "case e =>" and case "e: Throwable =>" are equivalent.
> 
> 
>   
> 
> 
> Diffs
> -----
> 
>   core/build.sbt c54cf44 
>   core/src/main/scala/kafka/Kafka.scala dafb1ee 
>   core/src/main/scala/kafka/admin/AddPartitionsCommand.scala 5757c32 
>   core/src/main/scala/kafka/admin/AdminUtils.scala c399bc7 
>   core/src/main/scala/kafka/admin/CreateTopicCommand.scala 21c1186 
>   core/src/main/scala/kafka/admin/DeleteTopicCommand.scala 3da4518 
>   core/src/main/scala/kafka/admin/ListTopicCommand.scala c760cc0 
>   core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala d5de5f3 
>   core/src/main/scala/kafka/admin/ReassignPartitionsCommand.scala aa61fa1 
>   core/src/main/scala/kafka/client/ClientUtils.scala cc526ec 
>   core/src/main/scala/kafka/cluster/Broker.scala b03dea2 
>   core/src/main/scala/kafka/consumer/ConsoleConsumer.scala 719beb5 
>   core/src/main/scala/kafka/consumer/ConsumerFetcherManager.scala fa6b213 
>   core/src/main/scala/kafka/consumer/SimpleConsumer.scala 4395fe3 
>   core/src/main/scala/kafka/consumer/TopicCount.scala c8e8406 
>   core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala e7a692a 
>   core/src/main/scala/kafka/consumer/ZookeeperTopicEventWatcher.scala df83baa 
>   core/src/main/scala/kafka/controller/ControllerChannelManager.scala ed1ce0b 
>   core/src/main/scala/kafka/controller/KafkaController.scala ab18b7a 
>   core/src/main/scala/kafka/controller/PartitionStateMachine.scala a084830 
>   core/src/main/scala/kafka/controller/ReplicaStateMachine.scala c964857 
>   core/src/main/scala/kafka/javaapi/FetchRequest.scala b475240 
>   core/src/main/scala/kafka/javaapi/Implicits.scala ee0a71d 
>   core/src/main/scala/kafka/javaapi/OffsetRequest.scala 1c77ff8 
>   core/src/main/scala/kafka/javaapi/TopicMetadata.scala 97b6dcd 
>   core/src/main/scala/kafka/javaapi/TopicMetadataRequest.scala 5f80df7 
>   core/src/main/scala/kafka/javaapi/consumer/ZookeeperConsumerConnector.scala 14c4c8a 
>   core/src/main/scala/kafka/javaapi/message/ByteBufferMessageSet.scala 0a95248 
>   core/src/main/scala/kafka/javaapi/producer/Producer.scala 7265328 
>   core/src/main/scala/kafka/log/LogManager.scala 4771d11 
>   core/src/main/scala/kafka/network/BoundedByteBufferReceive.scala cab1864 
>   core/src/main/scala/kafka/producer/SyncProducer.scala 306f200 
>   core/src/main/scala/kafka/producer/async/DefaultEventHandler.scala 2e36d3b 
>   core/src/main/scala/kafka/producer/async/ProducerSendThread.scala 2b41a49 
>   core/src/main/scala/kafka/server/AbstractFetcherThread.scala d5addb3 
>   core/src/main/scala/kafka/server/KafkaApis.scala cd02aab 
>   core/src/main/scala/kafka/server/KafkaServerStartable.scala 5be65e9 
>   core/src/main/scala/kafka/server/ReplicaManager.scala f551243 
>   core/src/main/scala/kafka/server/ZookeeperLeaderElector.scala f1f0625 
>   core/src/main/scala/kafka/tools/ImportZkOffsets.scala 55709b5 
>   core/src/main/scala/kafka/tools/JmxTool.scala 7e424e7 
>   core/src/main/scala/kafka/tools/MirrorMaker.scala 6fb545a 
>   core/src/main/scala/kafka/tools/SimpleConsumerShell.scala 3cfa384 
>   core/src/main/scala/kafka/utils/Annotations.scala 28269eb 
>   core/src/main/scala/kafka/utils/Annotations_2.8.scala PRE-CREATION 
>   core/src/main/scala/kafka/utils/Annotations_2.9+.scala PRE-CREATION 
>   core/src/main/scala/kafka/utils/Json.scala f80b2cc 
>   core/src/main/scala/kafka/utils/Mx4jLoader.scala 64d84cc 
>   core/src/main/scala/kafka/utils/Pool.scala 9a86eab 
>   core/src/main/scala/kafka/utils/Utils.scala e83eb5f 
>   core/src/main/scala/kafka/utils/ZkUtils.scala ca1ce12 
>   core/src/test/scala/unit/kafka/admin/AddPartitionsTest.scala 06be990 
>   core/src/test/scala/unit/kafka/admin/AdminTest.scala dc0013f 
>   core/src/test/scala/unit/kafka/consumer/ZookeeperConsumerConnectorTest.scala fcfc583 
>   core/src/test/scala/unit/kafka/javaapi/consumer/ZookeeperConsumerConnectorTest.scala 9f243f0 
>   core/src/test/scala/unit/kafka/javaapi/message/BaseMessageSetTestCases.scala abee11b 
>   core/src/test/scala/unit/kafka/log/LogTest.scala 4ed88e8 
>   core/src/test/scala/unit/kafka/metrics/KafkaTimerTest.scala fe5bc09 
>   core/src/test/scala/unit/kafka/producer/AsyncProducerTest.scala 1781bc0 
>   core/src/test/scala/unit/kafka/producer/ProducerTest.scala 29331db 
>   core/src/test/scala/unit/kafka/producer/SyncProducerTest.scala b3e89c3 
>   core/src/test/scala/unit/kafka/utils/TestUtils.scala 830608f 
>   perf/src/main/scala/kafka/perf/ConsumerPerformance.scala 3158a22 
>   project/Build.scala b3858f3 
> 
> Diff: https://reviews.apache.org/r/14013/diff/
> 
> 
> Testing
> -------
> 
> successfully performed cross build across all scala versions
> unit tests pass across all scala versions
> 
> 
> Thanks,
> 
> Christopher Freeman
> 
>


Re: Review Request 14013: Added support for Scala 2.10 builds while maintaining compatibility with 2.8.x

Posted by Neha Narkhede <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14013/#review25977
-----------------------------------------------------------


Overall, this is a great patch. Thanks for contributing it. I have some minor questions/suggestions. 


core/build.sbt
<https://reviews.apache.org/r/14013/#comment50698>

    Does Scala 2.10 require scalatest 1.9.1? 
    How about Scala 2.9.x ?



core/src/main/scala/kafka/javaapi/Implicits.scala
<https://reviews.apache.org/r/14013/#comment50699>

    Could you please add a comment above this API explaining why we need to do this ? The same one you have in ByteBufferMessageSet.scala



core/src/main/scala/kafka/utils/Pool.scala
<https://reviews.apache.org/r/14013/#comment50700>

    In all other places, we have limited the scope of the JavaConversions import. Could we also do the same here?



project/Build.scala
<https://reviews.apache.org/r/14013/#comment50701>

    This is a cool hack. I think we can get rid of this only if/when we drop support for Scala 2.8.x.


- Neha Narkhede


On Sept. 7, 2013, 11:35 p.m., Christopher Freeman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14013/
> -----------------------------------------------------------
> 
> (Updated Sept. 7, 2013, 11:35 p.m.)
> 
> 
> Review request for kafka and Neha Narkhede.
> 
> 
> Bugs: KAFKA-1046
>     https://issues.apache.org/jira/browse/KAFKA-1046
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> This RB contains necessary changes to the kafka source code to support Scala 2.10.x while still maintaining support for Scala 2.8.x
> 
> The existing cause of source incompatibility between 2.8.x and 2.10.x was primarily due to the use of implicit conversions in the scala.collection.JavaConversions object.  Many of them where deprecated (and replaced with differently named methods) and the deprecated methods where removed in 2.10.  I avoided the source incompatibility by importing the implicits with a wild card.
> 
> Scala annotations moved to a different package in 2.9 and the reference to the old location was removed in 2.10.  I couldn't think of a source compatible way to support both in Annotations.scala so since this source file is very small, I created two copies and configured sbt to use the one appropriate for when building for a particular Scala version.
> 
> I also typed many of the catch block case statements with :Throwable.  This change isn't required but starting with Scala 2.9, the compiler will emit warnings if the type is emitted.  Otherwise "case e =>" and case "e: Throwable =>" are equivalent.
> 
> 
>   
> 
> 
> Diffs
> -----
> 
>   core/build.sbt c54cf44 
>   core/src/main/scala/kafka/Kafka.scala dafb1ee 
>   core/src/main/scala/kafka/admin/AddPartitionsCommand.scala 5757c32 
>   core/src/main/scala/kafka/admin/AdminUtils.scala c399bc7 
>   core/src/main/scala/kafka/admin/CreateTopicCommand.scala 21c1186 
>   core/src/main/scala/kafka/admin/DeleteTopicCommand.scala 3da4518 
>   core/src/main/scala/kafka/admin/ListTopicCommand.scala c760cc0 
>   core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala d5de5f3 
>   core/src/main/scala/kafka/admin/ReassignPartitionsCommand.scala aa61fa1 
>   core/src/main/scala/kafka/client/ClientUtils.scala cc526ec 
>   core/src/main/scala/kafka/cluster/Broker.scala b03dea2 
>   core/src/main/scala/kafka/consumer/ConsoleConsumer.scala 719beb5 
>   core/src/main/scala/kafka/consumer/ConsumerFetcherManager.scala fa6b213 
>   core/src/main/scala/kafka/consumer/SimpleConsumer.scala 4395fe3 
>   core/src/main/scala/kafka/consumer/TopicCount.scala c8e8406 
>   core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala e7a692a 
>   core/src/main/scala/kafka/consumer/ZookeeperTopicEventWatcher.scala df83baa 
>   core/src/main/scala/kafka/controller/ControllerChannelManager.scala ed1ce0b 
>   core/src/main/scala/kafka/controller/KafkaController.scala ab18b7a 
>   core/src/main/scala/kafka/controller/PartitionStateMachine.scala a084830 
>   core/src/main/scala/kafka/controller/ReplicaStateMachine.scala c964857 
>   core/src/main/scala/kafka/javaapi/FetchRequest.scala b475240 
>   core/src/main/scala/kafka/javaapi/Implicits.scala ee0a71d 
>   core/src/main/scala/kafka/javaapi/OffsetRequest.scala 1c77ff8 
>   core/src/main/scala/kafka/javaapi/TopicMetadata.scala 97b6dcd 
>   core/src/main/scala/kafka/javaapi/TopicMetadataRequest.scala 5f80df7 
>   core/src/main/scala/kafka/javaapi/consumer/ZookeeperConsumerConnector.scala 14c4c8a 
>   core/src/main/scala/kafka/javaapi/message/ByteBufferMessageSet.scala 0a95248 
>   core/src/main/scala/kafka/javaapi/producer/Producer.scala 7265328 
>   core/src/main/scala/kafka/log/LogManager.scala 4771d11 
>   core/src/main/scala/kafka/network/BoundedByteBufferReceive.scala cab1864 
>   core/src/main/scala/kafka/producer/SyncProducer.scala 306f200 
>   core/src/main/scala/kafka/producer/async/DefaultEventHandler.scala 2e36d3b 
>   core/src/main/scala/kafka/producer/async/ProducerSendThread.scala 2b41a49 
>   core/src/main/scala/kafka/server/AbstractFetcherThread.scala d5addb3 
>   core/src/main/scala/kafka/server/KafkaApis.scala cd02aab 
>   core/src/main/scala/kafka/server/KafkaServerStartable.scala 5be65e9 
>   core/src/main/scala/kafka/server/ReplicaManager.scala f551243 
>   core/src/main/scala/kafka/server/ZookeeperLeaderElector.scala f1f0625 
>   core/src/main/scala/kafka/tools/ImportZkOffsets.scala 55709b5 
>   core/src/main/scala/kafka/tools/JmxTool.scala 7e424e7 
>   core/src/main/scala/kafka/tools/MirrorMaker.scala 6fb545a 
>   core/src/main/scala/kafka/tools/SimpleConsumerShell.scala 3cfa384 
>   core/src/main/scala/kafka/utils/Annotations.scala 28269eb 
>   core/src/main/scala/kafka/utils/Annotations_2.8.scala PRE-CREATION 
>   core/src/main/scala/kafka/utils/Annotations_2.9+.scala PRE-CREATION 
>   core/src/main/scala/kafka/utils/Json.scala f80b2cc 
>   core/src/main/scala/kafka/utils/Mx4jLoader.scala 64d84cc 
>   core/src/main/scala/kafka/utils/Pool.scala 9a86eab 
>   core/src/main/scala/kafka/utils/Utils.scala e83eb5f 
>   core/src/main/scala/kafka/utils/ZkUtils.scala ca1ce12 
>   core/src/test/scala/unit/kafka/admin/AddPartitionsTest.scala 06be990 
>   core/src/test/scala/unit/kafka/admin/AdminTest.scala dc0013f 
>   core/src/test/scala/unit/kafka/consumer/ZookeeperConsumerConnectorTest.scala fcfc583 
>   core/src/test/scala/unit/kafka/javaapi/consumer/ZookeeperConsumerConnectorTest.scala 9f243f0 
>   core/src/test/scala/unit/kafka/javaapi/message/BaseMessageSetTestCases.scala abee11b 
>   core/src/test/scala/unit/kafka/log/LogTest.scala 4ed88e8 
>   core/src/test/scala/unit/kafka/metrics/KafkaTimerTest.scala fe5bc09 
>   core/src/test/scala/unit/kafka/producer/AsyncProducerTest.scala 1781bc0 
>   core/src/test/scala/unit/kafka/producer/ProducerTest.scala 29331db 
>   core/src/test/scala/unit/kafka/producer/SyncProducerTest.scala b3e89c3 
>   core/src/test/scala/unit/kafka/utils/TestUtils.scala 830608f 
>   perf/src/main/scala/kafka/perf/ConsumerPerformance.scala 3158a22 
>   project/Build.scala b3858f3 
> 
> Diff: https://reviews.apache.org/r/14013/diff/
> 
> 
> Testing
> -------
> 
> successfully performed cross build across all scala versions
> unit tests pass across all scala versions
> 
> 
> Thanks,
> 
> Christopher Freeman
> 
>


Re: Review Request 14013: Added support for Scala 2.10 builds while maintaining compatibility with 2.8.x

Posted by Christopher Freeman <cf...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14013/
-----------------------------------------------------------

(Updated Sept. 7, 2013, 11:35 p.m.)


Review request for kafka and Neha Narkhede.


Changes
-------

replaced diff with one that is against the 0.8 branch


Bugs: KAFKA-1046
    https://issues.apache.org/jira/browse/KAFKA-1046


Repository: kafka


Description
-------

This RB contains necessary changes to the kafka source code to support Scala 2.10.x while still maintaining support for Scala 2.8.x

The existing cause of source incompatibility between 2.8.x and 2.10.x was primarily due to the use of implicit conversions in the scala.collection.JavaConversions object.  Many of them where deprecated (and replaced with differently named methods) and the deprecated methods where removed in 2.10.  I avoided the source incompatibility by importing the implicits with a wild card.

Scala annotations moved to a different package in 2.9 and the reference to the old location was removed in 2.10.  I couldn't think of a source compatible way to support both in Annotations.scala so since this source file is very small, I created two copies and configured sbt to use the one appropriate for when building for a particular Scala version.

I also typed many of the catch block case statements with :Throwable.  This change isn't required but starting with Scala 2.9, the compiler will emit warnings if the type is emitted.  Otherwise "case e =>" and case "e: Throwable =>" are equivalent.


  


Diffs (updated)
-----

  core/build.sbt c54cf44 
  core/src/main/scala/kafka/Kafka.scala dafb1ee 
  core/src/main/scala/kafka/admin/AddPartitionsCommand.scala 5757c32 
  core/src/main/scala/kafka/admin/AdminUtils.scala c399bc7 
  core/src/main/scala/kafka/admin/CreateTopicCommand.scala 21c1186 
  core/src/main/scala/kafka/admin/DeleteTopicCommand.scala 3da4518 
  core/src/main/scala/kafka/admin/ListTopicCommand.scala c760cc0 
  core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala d5de5f3 
  core/src/main/scala/kafka/admin/ReassignPartitionsCommand.scala aa61fa1 
  core/src/main/scala/kafka/client/ClientUtils.scala cc526ec 
  core/src/main/scala/kafka/cluster/Broker.scala b03dea2 
  core/src/main/scala/kafka/consumer/ConsoleConsumer.scala 719beb5 
  core/src/main/scala/kafka/consumer/ConsumerFetcherManager.scala fa6b213 
  core/src/main/scala/kafka/consumer/SimpleConsumer.scala 4395fe3 
  core/src/main/scala/kafka/consumer/TopicCount.scala c8e8406 
  core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala e7a692a 
  core/src/main/scala/kafka/consumer/ZookeeperTopicEventWatcher.scala df83baa 
  core/src/main/scala/kafka/controller/ControllerChannelManager.scala ed1ce0b 
  core/src/main/scala/kafka/controller/KafkaController.scala ab18b7a 
  core/src/main/scala/kafka/controller/PartitionStateMachine.scala a084830 
  core/src/main/scala/kafka/controller/ReplicaStateMachine.scala c964857 
  core/src/main/scala/kafka/javaapi/FetchRequest.scala b475240 
  core/src/main/scala/kafka/javaapi/Implicits.scala ee0a71d 
  core/src/main/scala/kafka/javaapi/OffsetRequest.scala 1c77ff8 
  core/src/main/scala/kafka/javaapi/TopicMetadata.scala 97b6dcd 
  core/src/main/scala/kafka/javaapi/TopicMetadataRequest.scala 5f80df7 
  core/src/main/scala/kafka/javaapi/consumer/ZookeeperConsumerConnector.scala 14c4c8a 
  core/src/main/scala/kafka/javaapi/message/ByteBufferMessageSet.scala 0a95248 
  core/src/main/scala/kafka/javaapi/producer/Producer.scala 7265328 
  core/src/main/scala/kafka/log/LogManager.scala 4771d11 
  core/src/main/scala/kafka/network/BoundedByteBufferReceive.scala cab1864 
  core/src/main/scala/kafka/producer/SyncProducer.scala 306f200 
  core/src/main/scala/kafka/producer/async/DefaultEventHandler.scala 2e36d3b 
  core/src/main/scala/kafka/producer/async/ProducerSendThread.scala 2b41a49 
  core/src/main/scala/kafka/server/AbstractFetcherThread.scala d5addb3 
  core/src/main/scala/kafka/server/KafkaApis.scala cd02aab 
  core/src/main/scala/kafka/server/KafkaServerStartable.scala 5be65e9 
  core/src/main/scala/kafka/server/ReplicaManager.scala f551243 
  core/src/main/scala/kafka/server/ZookeeperLeaderElector.scala f1f0625 
  core/src/main/scala/kafka/tools/ImportZkOffsets.scala 55709b5 
  core/src/main/scala/kafka/tools/JmxTool.scala 7e424e7 
  core/src/main/scala/kafka/tools/MirrorMaker.scala 6fb545a 
  core/src/main/scala/kafka/tools/SimpleConsumerShell.scala 3cfa384 
  core/src/main/scala/kafka/utils/Annotations.scala 28269eb 
  core/src/main/scala/kafka/utils/Annotations_2.8.scala PRE-CREATION 
  core/src/main/scala/kafka/utils/Annotations_2.9+.scala PRE-CREATION 
  core/src/main/scala/kafka/utils/Json.scala f80b2cc 
  core/src/main/scala/kafka/utils/Mx4jLoader.scala 64d84cc 
  core/src/main/scala/kafka/utils/Pool.scala 9a86eab 
  core/src/main/scala/kafka/utils/Utils.scala e83eb5f 
  core/src/main/scala/kafka/utils/ZkUtils.scala ca1ce12 
  core/src/test/scala/unit/kafka/admin/AddPartitionsTest.scala 06be990 
  core/src/test/scala/unit/kafka/admin/AdminTest.scala dc0013f 
  core/src/test/scala/unit/kafka/consumer/ZookeeperConsumerConnectorTest.scala fcfc583 
  core/src/test/scala/unit/kafka/javaapi/consumer/ZookeeperConsumerConnectorTest.scala 9f243f0 
  core/src/test/scala/unit/kafka/javaapi/message/BaseMessageSetTestCases.scala abee11b 
  core/src/test/scala/unit/kafka/log/LogTest.scala 4ed88e8 
  core/src/test/scala/unit/kafka/metrics/KafkaTimerTest.scala fe5bc09 
  core/src/test/scala/unit/kafka/producer/AsyncProducerTest.scala 1781bc0 
  core/src/test/scala/unit/kafka/producer/ProducerTest.scala 29331db 
  core/src/test/scala/unit/kafka/producer/SyncProducerTest.scala b3e89c3 
  core/src/test/scala/unit/kafka/utils/TestUtils.scala 830608f 
  perf/src/main/scala/kafka/perf/ConsumerPerformance.scala 3158a22 
  project/Build.scala b3858f3 

Diff: https://reviews.apache.org/r/14013/diff/


Testing
-------

successfully performed cross build across all scala versions
unit tests pass across all scala versions


Thanks,

Christopher Freeman