Flume, mail # dev - Review Request: FLUME-1773. File Channel worker thread should not be daemon

Review Request: FLUME-1773. File Channel worker thread should not be daemon

Hari Shreedharan 2012-12-08, 23:20

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

Review request for Flume.
Description
-------

Using a single threaded scheduled executor, which is stopped when the channel is killed.
This addresses bug FLUME-1773.
https://issues.apache.org/jira/browse/FLUME-1773
Diffs
-----

flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Log.java 6d1cf51

Diff: https://reviews.apache.org/r/8433/diff/
Testing
-------

All unit tests pass
Thanks,

Hari Shreedharan

Re: Review Request: FLUME-1773. File Channel worker thread should not be daemon

Hari Shreedharan 2012-12-08, 23:26

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

(Updated Dec. 8, 2012, 11:26 p.m.)
Review request for Flume.
Description
-------

Using a single threaded scheduled executor, which is stopped when the channel is killed.
This addresses bug FLUME-1773.
https://issues.apache.org/jira/browse/FLUME-1773
Diffs (updated)
-----

flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Log.java 6d1cf51

Diff: https://reviews.apache.org/r/8433/diff/
Testing
-------

All unit tests pass
Thanks,

Hari Shreedharan

Re: Review Request: FLUME-1773. File Channel worker thread should not be daemon

Hari Shreedharan 2012-12-10, 20:49

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

(Updated Dec. 10, 2012, 8:49 p.m.)
Review request for Flume.
Description
-------

Using a single threaded scheduled executor, which is stopped when the channel is killed.
This addresses bug FLUME-1773.
https://issues.apache.org/jira/browse/FLUME-1773
Diffs (updated)
-----

flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Log.java ea98e5d

Diff: https://reviews.apache.org/r/8433/diff/
Testing
-------

All unit tests pass
Thanks,

Hari Shreedharan

Re: Review Request: FLUME-1773. File Channel worker thread should not be daemon

Hari Shreedharan 2012-12-10, 20:51

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

(Updated Dec. 10, 2012, 8:51 p.m.)
Review request for Flume.
Description
-------

Using a single threaded scheduled executor, which is stopped when the channel is killed.
This addresses bug FLUME-1773.
https://issues.apache.org/jira/browse/FLUME-1773
Diffs (updated)
-----

flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Log.java ea98e5d

Diff: https://reviews.apache.org/r/8433/diff/
Testing
-------

All unit tests pass
Thanks,

Hari Shreedharan

Re: Review Request: FLUME-1773. File Channel worker thread should not be daemon

Hari Shreedharan 2012-12-10, 20:52

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

(Updated Dec. 10, 2012, 8:52 p.m.)
Review request for Flume.
Description
-------

Using a single threaded scheduled executor, which is stopped when the channel is killed.
This addresses bug FLUME-1773.
https://issues.apache.org/jira/browse/FLUME-1773
Diffs (updated)
-----

flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Log.java ea98e5d

Diff: https://reviews.apache.org/r/8433/diff/
Testing
-------

All unit tests pass
Thanks,

Hari Shreedharan

Re: Review Request: FLUME-1773. File Channel worker thread should not be daemon

Brock Noland 2012-12-10, 20:57

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8433/#review14260
-----------------------------------------------------------
Looks like a good patch. There are unit tests where we set the checkpoint interval to 1 millisecond and is then overridden to 1 second by the current code. I think we should probably update the unit tests or add the Math.max() back in? Thoughts?

- Brock Noland
On Dec. 10, 2012, 8:52 p.m., Hari Shreedharan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8433/
> -----------------------------------------------------------
>
> (Updated Dec. 10, 2012, 8:52 p.m.)
>
>
> Review request for Flume.
>
>
> Description
> -------
>
> Using a single threaded scheduled executor, which is stopped when the channel is killed.
>
>
> This addresses bug FLUME-1773.
> https://issues.apache.org/jira/browse/FLUME-1773
>
>
> Diffs
> -----
>
> flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Log.java ea98e5d
>
> Diff: https://reviews.apache.org/r/8433/diff/
>
>
> Testing
> -------
>
> All unit tests pass
>
>
> Thanks,
>
> Hari Shreedharan
>
>

Re: Review Request: FLUME-1773. File Channel worker thread should not be daemon

Hari Shreedharan 2012-12-10, 21:13

> On Dec. 10, 2012, 8:57 p.m., Brock Noland wrote:
> > Looks like a good patch. There are unit tests where we set the checkpoint interval to 1 millisecond and is then overridden to 1 second by the current code. I think we should probably update the unit tests or add the Math.max() back in? Thoughts?

Brock - the latest patch reverts the checkpoint interval to Millis
- Hari
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8433/#review14260
-----------------------------------------------------------
On Dec. 10, 2012, 8:52 p.m., Hari Shreedharan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8433/
> -----------------------------------------------------------
>
> (Updated Dec. 10, 2012, 8:52 p.m.)
>
>
> Review request for Flume.
>
>
> Description
> -------
>
> Using a single threaded scheduled executor, which is stopped when the channel is killed.
>
>
> This addresses bug FLUME-1773.
> https://issues.apache.org/jira/browse/FLUME-1773
>
>
> Diffs
> -----
>
> flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Log.java ea98e5d
>
> Diff: https://reviews.apache.org/r/8433/diff/
>
>
> Testing
> -------
>
> All unit tests pass
>
>
> Thanks,
>
> Hari Shreedharan
>
>

Re: Review Request: FLUME-1773. File Channel worker thread should not be daemon

Brock Noland 2012-12-10, 21:26

> On Dec. 10, 2012, 8:57 p.m., Brock Noland wrote:
> > Looks like a good patch. There are unit tests where we set the checkpoint interval to 1 millisecond and is then overridden to 1 second by the current code. I think we should probably update the unit tests or add the Math.max() back in? Thoughts?
>
> Hari Shreedharan wrote:
> Brock - the latest patch reverts the checkpoint interval to Millis

OK thanks! But there is still a change in behavior correct? The old behavior did not allow a checkpoint interval less than 1000 milliseconds. Many of our tests set this to 1MS relying on the Log class to force this to 1 second.
- Brock
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8433/#review14260
-----------------------------------------------------------
On Dec. 10, 2012, 8:52 p.m., Hari Shreedharan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8433/
> -----------------------------------------------------------
>
> (Updated Dec. 10, 2012, 8:52 p.m.)
>
>
> Review request for Flume.
>
>
> Description
> -------
>
> Using a single threaded scheduled executor, which is stopped when the channel is killed.
>
>
> This addresses bug FLUME-1773.
> https://issues.apache.org/jira/browse/FLUME-1773
>
>
> Diffs
> -----
>
> flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Log.java ea98e5d
>
> Diff: https://reviews.apache.org/r/8433/diff/
>
>
> Testing
> -------
>
> All unit tests pass
>
>
> Thanks,
>
> Hari Shreedharan
>
>

Re: Review Request: FLUME-1773. File Channel worker thread should not be daemon

Hari Shreedharan 2012-12-10, 21:28

> On Dec. 10, 2012, 8:57 p.m., Brock Noland wrote:
> > Looks like a good patch. There are unit tests where we set the checkpoint interval to 1 millisecond and is then overridden to 1 second by the current code. I think we should probably update the unit tests or add the Math.max() back in? Thoughts?
>
> Hari Shreedharan wrote:
> Brock - the latest patch reverts the checkpoint interval to Millis
>
> Brock Noland wrote:
> OK thanks! But there is still a change in behavior correct? The old behavior did not allow a checkpoint interval less than 1000 milliseconds. Many of our tests set this to 1MS relying on the Log class to force this to 1 second.

oh ok. I will add a math.max call.
- Hari
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8433/#review14260
-----------------------------------------------------------
On Dec. 10, 2012, 8:52 p.m., Hari Shreedharan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8433/
> -----------------------------------------------------------
>
> (Updated Dec. 10, 2012, 8:52 p.m.)
>
>
> Review request for Flume.
>
>
> Description
> -------
>
> Using a single threaded scheduled executor, which is stopped when the channel is killed.
>
>
> This addresses bug FLUME-1773.
> https://issues.apache.org/jira/browse/FLUME-1773
>
>
> Diffs
> -----
>
> flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Log.java ea98e5d
>
> Diff: https://reviews.apache.org/r/8433/diff/
>
>
> Testing
> -------
>
> All unit tests pass
>
>
> Thanks,
>
> Hari Shreedharan
>
>

Re: Review Request: FLUME-1773. File Channel worker thread should not be daemon

Hari Shreedharan 2012-12-10, 21:29

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

(Updated Dec. 10, 2012, 9:29 p.m.)
Review request for Flume.
Description
-------

Using a single threaded scheduled executor, which is stopped when the channel is killed.
This addresses bug FLUME-1773.
https://issues.apache.org/jira/browse/FLUME-1773
Diffs (updated)
-----

flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Log.java ea98e5d

Diff: https://reviews.apache.org/r/8433/diff/
Testing
-------

All unit tests pass
Thanks,

Hari Shreedharan

Re: Review Request: FLUME-1773. File Channel worker thread should not be daemon

Brock Noland 2012-12-10, 21:34

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

flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Log.java
<https://reviews.apache.org/r/8433/#comment30402>

Sorry I missed this before...why are we sysout'ing?
- Brock Noland
On Dec. 10, 2012, 9:29 p.m., Hari Shreedharan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8433/
> -----------------------------------------------------------
>
> (Updated Dec. 10, 2012, 9:29 p.m.)
>
>
> Review request for Flume.
>
>
> Description
> -------
>
> Using a single threaded scheduled executor, which is stopped when the channel is killed.
>
>
> This addresses bug FLUME-1773.
> https://issues.apache.org/jira/browse/FLUME-1773
>
>
> Diffs
> -----
>
> flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Log.java ea98e5d
>
> Diff: https://reviews.apache.org/r/8433/diff/
>
>
> Testing
> -------
>
> All unit tests pass
>
>
> Thanks,
>
> Hari Shreedharan
>
>

Re: Review Request: FLUME-1773. File Channel worker thread should not be daemon

Hari Shreedharan 2012-12-10, 21:37

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

flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Log.java
<https://reviews.apache.org/r/8433/#comment30403>

Since users could run Flume from a console (maybe in the background) and then kill it with Ctrl-C, then it is visible that Flume might take a few seconds to shutdown, so that they dont kill -9 it.
- Hari Shreedharan
On Dec. 10, 2012, 9:29 p.m., Hari Shreedharan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8433/
> -----------------------------------------------------------
>
> (Updated Dec. 10, 2012, 9:29 p.m.)
>
>
> Review request for Flume.
>
>
> Description
> -------
>
> Using a single threaded scheduled executor, which is stopped when the channel is killed.
>
>
> This addresses bug FLUME-1773.
> https://issues.apache.org/jira/browse/FLUME-1773
>
>
> Diffs
> -----
>
> flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Log.java ea98e5d
>
> Diff: https://reviews.apache.org/r/8433/diff/
>
>
> Testing
> -------
>
> All unit tests pass
>
>
> Thanks,
>
> Hari Shreedharan
>
>

Re: Review Request: FLUME-1773. File Channel worker thread should not be daemon

Brock Noland 2012-12-10, 23:43

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

Ship it!
Ship It!

- Brock Noland
On Dec. 10, 2012, 9:29 p.m., Hari Shreedharan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8433/
> -----------------------------------------------------------
>
> (Updated Dec. 10, 2012, 9:29 p.m.)
>
>
> Review request for Flume.
>
>
> Description
> -------
>
> Using a single threaded scheduled executor, which is stopped when the channel is killed.
>
>
> This addresses bug FLUME-1773.
> https://issues.apache.org/jira/browse/FLUME-1773
>
>
> Diffs
> -----
>
> flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Log.java ea98e5d
>
> Diff: https://reviews.apache.org/r/8433/diff/
>
>
> Testing
> -------
>
> All unit tests pass
>
>
> Thanks,
>
> Hari Shreedharan
>
>