Re: Review Request 24214: Patch for KAFKA-1374

2015-05-19 Thread Joel Koshy
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24214/#review84278 --- Thanks for the updated patch. This looks good. I ended up rebasing w

Re: Review Request 24214: Patch for KAFKA-1374

2015-05-18 Thread Manikumar Reddy O
> On May 12, 2015, 2:01 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/log/LogCleaner.scala, line 409 > > > > > > I would suggest one of two options over this (i.e., instead of two > > helper methods) > > -

Re: Review Request 24214: Patch for KAFKA-1374

2015-05-18 Thread Manikumar Reddy O
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24214/ --- (Updated May 18, 2015, 5:29 p.m.) Review request for kafka. Bugs: KAFKA-1374

Re: Review Request 24214: Patch for KAFKA-1374

2015-05-12 Thread Joel Koshy
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24214/#review83392 --- Sorry for the delay. Overall, this looks good. As discussed earlier

Re: Review Request 24214: Patch for KAFKA-1374

2015-01-18 Thread Eric Olander
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24214/#review68569 --- core/src/test/scala/unit/kafka/log/LogCleanerIntegrationTest.scala

Re: Review Request 24214: Patch for KAFKA-1374

2015-01-17 Thread Manikumar Reddy O
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24214/ --- (Updated Jan. 17, 2015, 6:53 p.m.) Review request for kafka. Bugs: KAFKA-1374

Re: Review Request 24214: Patch for KAFKA-1374

2015-01-17 Thread Manikumar Reddy O
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24214/ --- (Updated Jan. 17, 2015, 6:51 p.m.) Review request for kafka. Bugs: KAFKA-1374

Re: Review Request 24214: Patch for KAFKA-1374

2014-10-03 Thread Manikumar Reddy O
> On Aug. 18, 2014, 5:32 p.m., Joel Koshy wrote: > > I should be able to review this later today. However, as Jun also mentioned > > can you please run the stress test? When I was working on the original > > (WIP) patch it worked but eventually failed (due to various reasons such as > > corrup

Re: Review Request 24214: Patch for KAFKA-1374

2014-10-03 Thread Manikumar Reddy O
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24214/ --- (Updated Oct. 3, 2014, 1:50 p.m.) Review request for kafka. Bugs: KAFKA-1374

Re: Review Request 24214: Patch for KAFKA-1374

2014-10-03 Thread Manikumar Reddy O
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24214/ --- (Updated Oct. 3, 2014, 1:22 p.m.) Review request for kafka. Bugs: KAFKA-1374

Re: Review Request 24214: Patch for KAFKA-1374

2014-09-23 Thread Manikumar Reddy O
> On Aug. 18, 2014, 5:32 p.m., Joel Koshy wrote: > > I should be able to review this later today. However, as Jun also mentioned > > can you please run the stress test? When I was working on the original > > (WIP) patch it worked but eventually failed (due to various reasons such as > > corrup

Re: Review Request 24214: Patch for KAFKA-1374

2014-09-23 Thread Manikumar Reddy O
> On Aug. 18, 2014, 5:21 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/log/LogCleaner.scala, lines 436-438 > > > > > > Hmm, I think the original approach of throwing an exception is probably > > better. When ha

Re: Review Request 24214: Patch for KAFKA-1374

2014-09-23 Thread Manikumar Reddy O
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24214/ --- (Updated Sept. 23, 2014, 4:20 p.m.) Review request for kafka. Bugs: KAFKA-137

Re: Review Request 24214: Patch for KAFKA-1374

2014-08-18 Thread Joel Koshy
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24214/#review50901 --- I should be able to review this later today. However, as Jun also me

Re: Review Request 24214: Patch for KAFKA-1374

2014-08-18 Thread Jun Rao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24214/#review50893 --- Thanks for the patch. Looks good overall. Could you run the stress

Re: Review Request 24214: Patch for KAFKA-1374

2014-08-12 Thread Manikumar Reddy O
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24214/ --- (Updated Aug. 12, 2014, 4:57 p.m.) Review request for kafka. Bugs: KAFKA-1374

Re: Review Request 24214: Patch for KAFKA-1374

2014-08-12 Thread Manikumar Reddy O
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24214/ --- (Updated Aug. 12, 2014, 4:55 p.m.) Review request for kafka. Bugs: KAFKA-1374

Re: Review Request 24214: Patch for KAFKA-1374

2014-08-11 Thread Manikumar Reddy O
> On Aug. 10, 2014, 11:44 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/log/LogCleaner.scala, lines 400-420 > > > > > > Thinking about this a bit more. I am wondering if it would be better if > > we introduce a p

Re: Review Request 24214: Patch for KAFKA-1374

2014-08-10 Thread Manikumar Reddy O
> On Aug. 10, 2014, 11:44 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/log/LogCleaner.scala, lines 400-420 > > > > > > Thinking about this a bit more. I am wondering if it would be better if > > we introduce a p

Re: Review Request 24214: Patch for KAFKA-1374

2014-08-10 Thread Jun Rao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24214/#review50128 --- core/src/main/scala/kafka/log/LogCleaner.scala

Re: Review Request 24214: Patch for KAFKA-1374

2014-08-09 Thread Manikumar Reddy O
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24214/ --- (Updated Aug. 9, 2014, 10:51 a.m.) Review request for kafka. Bugs: KAFKA-1374

Re: Review Request 24214: Patch for KAFKA-1374

2014-08-09 Thread Manikumar Reddy O
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24214/ --- (Updated Aug. 9, 2014, 10:37 a.m.) Review request for kafka. Bugs: KAFKA-1374

Re: Review Request 24214: Patch for KAFKA-1374

2014-08-09 Thread Manikumar Reddy O
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24214/ --- (Updated Aug. 9, 2014, 10:30 a.m.) Review request for kafka. Bugs: KAFKA-1374

Re: Review Request 24214: Patch for KAFKA-1374

2014-08-07 Thread Jun Rao
> On Aug. 6, 2014, 4:29 a.m., Jun Rao wrote: > > core/src/main/scala/kafka/log/LogCleaner.scala, lines 500-506 > > > > > > Could we use Compressor.putRecord? Then,we don't have to worry about > > the details of the me

Re: Review Request 24214: Patch for KAFKA-1374

2014-08-07 Thread Manikumar Reddy O
> On Aug. 6, 2014, 4:29 a.m., Jun Rao wrote: > > core/src/main/scala/kafka/log/LogCleaner.scala, lines 500-506 > > > > > > Could we use Compressor.putRecord? Then,we don't have to worry about > > the details of the me

Re: Review Request 24214: Patch for KAFKA-1374

2014-08-05 Thread Jun Rao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24214/#review49611 --- Thanks for the patch. Some comments below. core/src/main/scala/kaf

Review Request 24214: Patch for KAFKA-1374

2014-08-03 Thread Manikumar Reddy O
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24214/ --- Review request for kafka. Bugs: KAFKA-1374 https://issues.apache.org/jira/b