You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Guozhang Wang <gu...@linkedin.com> on 2014/04/25 01:26:09 UTC

Re: Review Request 20616: Fix KAFKA-1409: Address Jun's comments round three

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

(Updated April 24, 2014, 11:26 p.m.)


Review request for kafka.


Summary (updated)
-----------------

Fix KAFKA-1409: Address Jun's comments round three


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


Repository: kafka


Description (updated)
-------

Update comments


Incorporate Jun's comments round three


Fix KAFKA-1409: Address Jun's comments round two


Diffs (updated)
-----

  core/src/main/scala/kafka/cluster/Partition.scala c08eab0bc35aa2b0caa0dfc1987b4face4315d3b 
  core/src/main/scala/kafka/log/Log.scala 46df8d99d977a3b010a9b9f4698187fa9bfb2498 
  core/src/main/scala/kafka/message/MessageSet.scala a1b5c63b4d9deaa2556427d995c41841838cf581 
  core/src/main/scala/kafka/server/KafkaApis.scala bb0359d933356b8fcc2c8df22ce6ea9cb838d6a2 
  core/src/main/scala/kafka/server/KafkaRequestHandler.scala f11f6e2c82b016f4dd3e92466ae1d2ede19feb30 

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


Testing
-------


Thanks,

Guozhang Wang

Re: Review Request 20616: Fix KAFKA-1409: Address Jun and Neha's comments

Posted by Guozhang Wang <gu...@linkedin.com>.

> On April 25, 2014, 5:32 p.m., Jun Rao wrote:
> > core/src/main/scala/kafka/log/Log.scala, lines 274-275
> > <https://reviews.apache.org/r/20616/diff/6/?file=568559#file568559line274>
> >
> >     entrySNnnnNize?

Never search for classes while doing "git commit" .... sorry about that.


- Guozhang


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


On April 25, 2014, 5:22 p.m., Guozhang Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20616/
> -----------------------------------------------------------
> 
> (Updated April 25, 2014, 5:22 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1409
>     https://issues.apache.org/jira/browse/KAFKA-1409
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> Address Neha's comments
> 
> 
> Update comments
> 
> 
> Incorporate Jun's comments round three
> 
> 
> Fix KAFKA-1409: Address Jun's comments round two
> 
> 
> Diffs
> -----
> 
>   core/src/main/scala/kafka/cluster/Partition.scala c08eab0bc35aa2b0caa0dfc1987b4face4315d3b 
>   core/src/main/scala/kafka/log/Log.scala 46df8d99d977a3b010a9b9f4698187fa9bfb2498 
>   core/src/main/scala/kafka/message/MessageSet.scala a1b5c63b4d9deaa2556427d995c41841838cf581 
>   core/src/main/scala/kafka/server/KafkaApis.scala bb0359d933356b8fcc2c8df22ce6ea9cb838d6a2 
>   core/src/main/scala/kafka/server/KafkaRequestHandler.scala f11f6e2c82b016f4dd3e92466ae1d2ede19feb30 
> 
> Diff: https://reviews.apache.org/r/20616/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Guozhang Wang
> 
>

Re: Review Request 20616: Fix KAFKA-1409: Address Jun and Neha's comments

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



core/src/main/scala/kafka/log/Log.scala
<https://reviews.apache.org/r/20616/#comment74934>

    entrySnnnnNize?



core/src/main/scala/kafka/log/Log.scala
<https://reviews.apache.org/r/20616/#comment74936>

    entrySNnnnNize?


- Jun Rao


On April 25, 2014, 5:22 p.m., Guozhang Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20616/
> -----------------------------------------------------------
> 
> (Updated April 25, 2014, 5:22 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1409
>     https://issues.apache.org/jira/browse/KAFKA-1409
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> Address Neha's comments
> 
> 
> Update comments
> 
> 
> Incorporate Jun's comments round three
> 
> 
> Fix KAFKA-1409: Address Jun's comments round two
> 
> 
> Diffs
> -----
> 
>   core/src/main/scala/kafka/cluster/Partition.scala c08eab0bc35aa2b0caa0dfc1987b4face4315d3b 
>   core/src/main/scala/kafka/log/Log.scala 46df8d99d977a3b010a9b9f4698187fa9bfb2498 
>   core/src/main/scala/kafka/message/MessageSet.scala a1b5c63b4d9deaa2556427d995c41841838cf581 
>   core/src/main/scala/kafka/server/KafkaApis.scala bb0359d933356b8fcc2c8df22ce6ea9cb838d6a2 
>   core/src/main/scala/kafka/server/KafkaRequestHandler.scala f11f6e2c82b016f4dd3e92466ae1d2ede19feb30 
> 
> Diff: https://reviews.apache.org/r/20616/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Guozhang Wang
> 
>

Re: Review Request 20616: Fix KAFKA-1409: Address Jun and Neha's comments

Posted by Guozhang Wang <gu...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20616/
-----------------------------------------------------------

(Updated April 25, 2014, 5:44 p.m.)


Review request for kafka.


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


Repository: kafka


Description (updated)
-------

typo fix


Address Neha's comments


Update comments


Incorporate Jun's comments round three


Fix KAFKA-1409: Address Jun's comments round two


Diffs (updated)
-----

  core/src/main/scala/kafka/cluster/Partition.scala c08eab0bc35aa2b0caa0dfc1987b4face4315d3b 
  core/src/main/scala/kafka/log/Log.scala 46df8d99d977a3b010a9b9f4698187fa9bfb2498 
  core/src/main/scala/kafka/message/MessageSet.scala a1b5c63b4d9deaa2556427d995c41841838cf581 
  core/src/main/scala/kafka/server/KafkaApis.scala bb0359d933356b8fcc2c8df22ce6ea9cb838d6a2 
  core/src/main/scala/kafka/server/KafkaRequestHandler.scala f11f6e2c82b016f4dd3e92466ae1d2ede19feb30 

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


Testing
-------


Thanks,

Guozhang Wang

Re: Review Request 20616: Fix KAFKA-1409: Address Jun and Neha's comments

Posted by Guozhang Wang <gu...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20616/
-----------------------------------------------------------

(Updated April 25, 2014, 5:22 p.m.)


Review request for kafka.


Summary (updated)
-----------------

Fix KAFKA-1409: Address Jun and Neha's comments


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


Repository: kafka


Description (updated)
-------

Address Neha's comments


Update comments


Incorporate Jun's comments round three


Fix KAFKA-1409: Address Jun's comments round two


Diffs (updated)
-----

  core/src/main/scala/kafka/cluster/Partition.scala c08eab0bc35aa2b0caa0dfc1987b4face4315d3b 
  core/src/main/scala/kafka/log/Log.scala 46df8d99d977a3b010a9b9f4698187fa9bfb2498 
  core/src/main/scala/kafka/message/MessageSet.scala a1b5c63b4d9deaa2556427d995c41841838cf581 
  core/src/main/scala/kafka/server/KafkaApis.scala bb0359d933356b8fcc2c8df22ce6ea9cb838d6a2 
  core/src/main/scala/kafka/server/KafkaRequestHandler.scala f11f6e2c82b016f4dd3e92466ae1d2ede19feb30 

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


Testing
-------


Thanks,

Guozhang Wang

Re: Review Request 20616: Fix KAFKA-1409: Address Jun's comments round three

Posted by Guozhang Wang <gu...@linkedin.com>.

> On April 25, 2014, 12:24 a.m., Neha Narkhede wrote:
> > core/src/main/scala/kafka/log/Log.scala, line 275
> > <https://reviews.apache.org/r/20616/diff/5/?file=568178#file568178line275>
> >
> >     Can we compute messages.sizeInBytes just once instead of twice?

messages.sizeInBytes just calls buffer.limit, which does not do any computation.


- Guozhang


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


On April 24, 2014, 11:26 p.m., Guozhang Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20616/
> -----------------------------------------------------------
> 
> (Updated April 24, 2014, 11:26 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1409
>     https://issues.apache.org/jira/browse/KAFKA-1409
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> Update comments
> 
> 
> Incorporate Jun's comments round three
> 
> 
> Fix KAFKA-1409: Address Jun's comments round two
> 
> 
> Diffs
> -----
> 
>   core/src/main/scala/kafka/cluster/Partition.scala c08eab0bc35aa2b0caa0dfc1987b4face4315d3b 
>   core/src/main/scala/kafka/log/Log.scala 46df8d99d977a3b010a9b9f4698187fa9bfb2498 
>   core/src/main/scala/kafka/message/MessageSet.scala a1b5c63b4d9deaa2556427d995c41841838cf581 
>   core/src/main/scala/kafka/server/KafkaApis.scala bb0359d933356b8fcc2c8df22ce6ea9cb838d6a2 
>   core/src/main/scala/kafka/server/KafkaRequestHandler.scala f11f6e2c82b016f4dd3e92466ae1d2ede19feb30 
> 
> Diff: https://reviews.apache.org/r/20616/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Guozhang Wang
> 
>

Re: Review Request 20616: Fix KAFKA-1409: Address Jun's comments round three

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



core/src/main/scala/kafka/log/Log.scala
<https://reviews.apache.org/r/20616/#comment74859>

    Can we compute messages.sizeInBytes just once instead of twice?



core/src/main/scala/kafka/log/Log.scala
<https://reviews.apache.org/r/20616/#comment74861>

    same here



core/src/main/scala/kafka/log/Log.scala
<https://reviews.apache.org/r/20616/#comment74862>

    You can use messageSize instead of recomputing the message entry size again


- Neha Narkhede


On April 24, 2014, 11:26 p.m., Guozhang Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20616/
> -----------------------------------------------------------
> 
> (Updated April 24, 2014, 11:26 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1409
>     https://issues.apache.org/jira/browse/KAFKA-1409
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> Update comments
> 
> 
> Incorporate Jun's comments round three
> 
> 
> Fix KAFKA-1409: Address Jun's comments round two
> 
> 
> Diffs
> -----
> 
>   core/src/main/scala/kafka/cluster/Partition.scala c08eab0bc35aa2b0caa0dfc1987b4face4315d3b 
>   core/src/main/scala/kafka/log/Log.scala 46df8d99d977a3b010a9b9f4698187fa9bfb2498 
>   core/src/main/scala/kafka/message/MessageSet.scala a1b5c63b4d9deaa2556427d995c41841838cf581 
>   core/src/main/scala/kafka/server/KafkaApis.scala bb0359d933356b8fcc2c8df22ce6ea9cb838d6a2 
>   core/src/main/scala/kafka/server/KafkaRequestHandler.scala f11f6e2c82b016f4dd3e92466ae1d2ede19feb30 
> 
> Diff: https://reviews.apache.org/r/20616/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Guozhang Wang
> 
>