You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@kafka.apache.org by Onur Karaman <ok...@linkedin.com> on 2015/06/01 21:03:45 UTC

Re: Review Request 34734: Patch for KAFKA-2226

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


Ran the patch with the following command 60+ times and didn't see an NPE (earlier attempts before this patch took anywhere from 5 - 30 attempts to hit the NPE):
```
./bin/kafka-run-class.sh kafka.TestPurgatoryPerformance --key-space-size 100000 --keys 3 --num 100000 --pct50 50 --pct75 75 --rate 1000 --size 50 --timeout 20
```

As a side note, I think the timing wheel design would be simpler if:
1. we allow TimerTasks to only be run at most once
2. we force TimerTask to have exactly one TimerTaskEntry and TimerTaskEntry to only ever belong to exactly one TimerTask (just make the TimerTaskEntry in the TimerTask's constructor).

- Onur Karaman


On May 29, 2015, 10:10 p.m., Yasuhiro Matsuda wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34734/
> -----------------------------------------------------------
> 
> (Updated May 29, 2015, 10:10 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-2226
>     https://issues.apache.org/jira/browse/KAFKA-2226
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> fix a race condition in TimerTaskEntry.remove
> 
> 
> Diffs
> -----
> 
>   core/src/main/scala/kafka/utils/timer/Timer.scala b8cde820a770a4e894804f1c268b24b529940650 
>   core/src/main/scala/kafka/utils/timer/TimerTask.scala 3407138115d579339ffb6b00e32e38c984ac5d6e 
>   core/src/main/scala/kafka/utils/timer/TimerTaskList.scala e7a96570ddc2367583d6d5590628db7e7f6ba39b 
>   core/src/main/scala/kafka/utils/timer/TimingWheel.scala e92aba3844dbf3372182e14536a5d98cf3366d73 
> 
> Diff: https://reviews.apache.org/r/34734/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Yasuhiro Matsuda
> 
>