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

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

If atomic rename fails, rename the current meta file to old, rename tmp to meta and then delete old.
This addresses bug FLUME-1699.
    https://issues.apache.org/jira/browse/FLUME-1699
Diffs
-----

  flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/LogFileV3.java f768d23

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

All unit tests pass
Thanks,

Hari Shreedharan

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8087/#review13525
-----------------------------------------------------------
why not simplify it to ..
//  delete log.meta  
//  log.meta.tmp -> log.meta
- Roshan Naik
On Nov. 16, 2012, 7:26 p.m., Hari Shreedharan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8087/
> -----------------------------------------------------------
>
> (Updated Nov. 16, 2012, 7:26 p.m.)
>
>
> Review request for Flume.
>
>
> Description
> -------
>
> If atomic rename fails, rename the current meta file to old, rename tmp to meta and then delete old.
>
>
> This addresses bug FLUME-1699.
>     https://issues.apache.org/jira/browse/FLUME-1699
>
>
> Diffs
> -----
>
>   flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/LogFileV3.java f768d23
>
> Diff: https://reviews.apache.org/r/8087/diff/
>
>
> Testing
> -------
>
> All unit tests pass
>
>
> Thanks,
>
> Hari Shreedharan
>
>

> On Nov. 16, 2012, 7:48 p.m., Roshan Naik wrote:
> > why not simplify it to ..
> >
> >
> > //  delete log.meta  
> > //  log.meta.tmp -> log.meta
> >
>
> Hari Shreedharan wrote:
>     I am not entirely sure how all platforms handle renames. Depending on implementation, though highly unlikely, if the system crashes in the middle of a rename, the tmp file might disappear without the ".meta" file being created. Leaving the old file guarantees that either the old or the new file is available at any point in time.
>    
>     I am just being paranoid about it since I don't know how different operating systems implement renames (for example, Windows does not seem to give many guarantees about durability in the event of a crash during a rename)
>
> Mike Percy wrote:
>     Do we ever look for log.meta.old? If not, I think we should look for it @ startup time if log.meta does not exist.
>    
>     Roshan: If we lose the meta file on an encrypted file channel, we are screwed because the meta file contains the initialization vector for AES/CTR/Nopadding. The data will be irrecovably lost.
>

Hmm, you are right. I was just think of leaving it as is, and expect that the user would fix it by doing the renames manually as is. Will update the patch. Thanks Mike, Roshan.
- Hari
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8087/#review13525
-----------------------------------------------------------
On Nov. 16, 2012, 7:26 p.m., Hari Shreedharan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8087/
> -----------------------------------------------------------
>
> (Updated Nov. 16, 2012, 7:26 p.m.)
>
>
> Review request for Flume.
>
>
> Description
> -------
>
> If atomic rename fails, rename the current meta file to old, rename tmp to meta and then delete old.
>
>
> This addresses bug FLUME-1699.
>     https://issues.apache.org/jira/browse/FLUME-1699
>
>
> Diffs
> -----
>
>   flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/LogFileV3.java f768d23
>
> Diff: https://reviews.apache.org/r/8087/diff/
>
>
> Testing
> -------
>
> All unit tests pass
>
>
> Thanks,
>
> Hari Shreedharan
>
>

> On Nov. 16, 2012, 7:48 p.m., Roshan Naik wrote:
> > why not simplify it to ..
> >
> >
> > //  delete log.meta  
> > //  log.meta.tmp -> log.meta
> >
>
> Hari Shreedharan wrote:
>     I am not entirely sure how all platforms handle renames. Depending on implementation, though highly unlikely, if the system crashes in the middle of a rename, the tmp file might disappear without the ".meta" file being created. Leaving the old file guarantees that either the old or the new file is available at any point in time.
>    
>     I am just being paranoid about it since I don't know how different operating systems implement renames (for example, Windows does not seem to give many guarantees about durability in the event of a crash during a rename)

Do we ever look for log.meta.old? If not, I think we should look for it @ startup time if log.meta does not exist.

Roshan: If we lose the meta file on an encrypted file channel, we are screwed because the meta file contains the initialization vector for AES/CTR/Nopadding. The data will be irrecovably lost.
- Mike
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8087/#review13525
-----------------------------------------------------------
On Nov. 16, 2012, 7:26 p.m., Hari Shreedharan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8087/
> -----------------------------------------------------------
>
> (Updated Nov. 16, 2012, 7:26 p.m.)
>
>
> Review request for Flume.
>
>
> Description
> -------
>
> If atomic rename fails, rename the current meta file to old, rename tmp to meta and then delete old.
>
>
> This addresses bug FLUME-1699.
>     https://issues.apache.org/jira/browse/FLUME-1699
>
>
> Diffs
> -----
>
>   flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/LogFileV3.java f768d23
>
> Diff: https://reviews.apache.org/r/8087/diff/
>
>
> Testing
> -------
>
> All unit tests pass
>
>
> Thanks,
>
> Hari Shreedharan
>
>

> On Nov. 16, 2012, 7:48 p.m., Roshan Naik wrote:
> > why not simplify it to ..
> >
> >
> > //  delete log.meta  
> > //  log.meta.tmp -> log.meta
> >

I am not entirely sure how all platforms handle renames. Depending on implementation, though highly unlikely, if the system crashes in the middle of a rename, the tmp file might disappear without the ".meta" file being created. Leaving the old file guarantees that either the old or the new file is available at any point in time.

I am just being paranoid about it since I don't know how different operating systems implement renames (for example, Windows does not seem to give many guarantees about durability in the event of a crash during a rename)
- Hari
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8087/#review13525
-----------------------------------------------------------
On Nov. 16, 2012, 7:26 p.m., Hari Shreedharan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8087/
> -----------------------------------------------------------
>
> (Updated Nov. 16, 2012, 7:26 p.m.)
>
>
> Review request for Flume.
>
>
> Description
> -------
>
> If atomic rename fails, rename the current meta file to old, rename tmp to meta and then delete old.
>
>
> This addresses bug FLUME-1699.
>     https://issues.apache.org/jira/browse/FLUME-1699
>
>
> Diffs
> -----
>
>   flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/LogFileV3.java f768d23
>
> Diff: https://reviews.apache.org/r/8087/diff/
>
>
> Testing
> -------
>
> All unit tests pass
>
>
> Thanks,
>
> Hari Shreedharan
>
>

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

(Updated Nov. 19, 2012, 4:42 a.m.)
Review request for Flume.
Changes
-------

Added code to handle picking up the temp or old files during startup. Also added unit tests for this.
Description
-------

If atomic rename fails, rename the current meta file to old, rename tmp to meta and then delete old.
This addresses bug FLUME-1699.
    https://issues.apache.org/jira/browse/FLUME-1699
Diffs (updated)
-----

  flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/LogFileFactory.java 82e14b0
  flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/LogFileV3.java f768d23
  flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Serialization.java 6b0eeb3
  flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestLogFile.java 4b69698

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

All unit tests pass
Thanks,

Hari Shreedharan

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

(Updated Nov. 19, 2012, 4:53 a.m.)
Review request for Flume and Brock Noland.
Description
-------

If atomic rename fails, rename the current meta file to old, rename tmp to meta and then delete old.
This addresses bug FLUME-1699.
    https://issues.apache.org/jira/browse/FLUME-1699
Diffs
-----

  flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/LogFileFactory.java 82e14b0
  flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/LogFileV3.java f768d23
  flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Serialization.java 6b0eeb3
  flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestLogFile.java 4b69698

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

All unit tests pass
Thanks,

Hari Shreedharan

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

flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/LogFileFactory.java
<https://reviews.apache.org/r/8087/#comment29141>

    Good catch, Brock! Fixing now.
- Hari Shreedharan
On Nov. 19, 2012, 4:53 a.m., Hari Shreedharan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8087/
> -----------------------------------------------------------
>
> (Updated Nov. 19, 2012, 4:53 a.m.)
>
>
> Review request for Flume and Brock Noland.
>
>
> Description
> -------
>
> If atomic rename fails, rename the current meta file to old, rename tmp to meta and then delete old.
>
>
> This addresses bug FLUME-1699.
>     https://issues.apache.org/jira/browse/FLUME-1699
>
>
> Diffs
> -----
>
>   flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/LogFileFactory.java 82e14b0
>   flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/LogFileV3.java f768d23
>   flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Serialization.java 6b0eeb3
>   flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestLogFile.java 4b69698
>
> Diff: https://reviews.apache.org/r/8087/diff/
>
>
> Testing
> -------
>
> All unit tests pass
>
>
> Thanks,
>
> Hari Shreedharan
>
>

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8087/#review13576
-----------------------------------------------------------
Looks quite good. Just one issue below
flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/LogFileFactory.java
<https://reviews.apache.org/r/8087/#comment29140>

    If either of these renames fail, we have set the hasMeta flag to true but no .meta file exists.
- Brock Noland
On Nov. 19, 2012, 4:53 a.m., Hari Shreedharan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8087/
> -----------------------------------------------------------
>
> (Updated Nov. 19, 2012, 4:53 a.m.)
>
>
> Review request for Flume and Brock Noland.
>
>
> Description
> -------
>
> If atomic rename fails, rename the current meta file to old, rename tmp to meta and then delete old.
>
>
> This addresses bug FLUME-1699.
>     https://issues.apache.org/jira/browse/FLUME-1699
>
>
> Diffs
> -----
>
>   flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/LogFileFactory.java 82e14b0
>   flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/LogFileV3.java f768d23
>   flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Serialization.java 6b0eeb3
>   flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestLogFile.java 4b69698
>
> Diff: https://reviews.apache.org/r/8087/diff/
>
>
> Testing
> -------
>
> All unit tests pass
>
>
> Thanks,
>
> Hari Shreedharan
>
>

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

(Updated Nov. 19, 2012, 7:41 p.m.)
Review request for Flume and Brock Noland.
Description
-------

If atomic rename fails, rename the current meta file to old, rename tmp to meta and then delete old.
This addresses bug FLUME-1699.
    https://issues.apache.org/jira/browse/FLUME-1699
Diffs (updated)
-----

  flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/LogFileFactory.java 82e14b0
  flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/LogFileV3.java f768d23
  flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Serialization.java 6b0eeb3
  flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestLogFile.java 4b69698

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

All unit tests pass
Thanks,

Hari Shreedharan

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

(Updated Nov. 19, 2012, 7:56 p.m.)
Review request for Flume and Brock Noland.
Description
-------

If atomic rename fails, rename the current meta file to old, rename tmp to meta and then delete old.
This addresses bug FLUME-1699.
    https://issues.apache.org/jira/browse/FLUME-1699
Diffs (updated)
-----

  flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/LogFileFactory.java 82e14b0
  flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/LogFileV3.java f768d23
  flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Serialization.java 6b0eeb3
  flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestLogFile.java 4b69698

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

All unit tests pass
Thanks,

Hari Shreedharan

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

Ship it!
Ship It!

- Brock Noland
On Nov. 19, 2012, 7:56 p.m., Hari Shreedharan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8087/
> -----------------------------------------------------------
>
> (Updated Nov. 19, 2012, 7:56 p.m.)
>
>
> Review request for Flume and Brock Noland.
>
>
> Description
> -------
>
> If atomic rename fails, rename the current meta file to old, rename tmp to meta and then delete old.
>
>
> This addresses bug FLUME-1699.
>     https://issues.apache.org/jira/browse/FLUME-1699
>
>
> Diffs
> -----
>
>   flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/LogFileFactory.java 82e14b0
>   flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/LogFileV3.java f768d23
>   flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Serialization.java 6b0eeb3
>   flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestLogFile.java 4b69698
>
> Diff: https://reviews.apache.org/r/8087/diff/
>
>
> Testing
> -------
>
> All unit tests pass
>
>
> Thanks,
>
> Hari Shreedharan
>
>

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

flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/LogFileFactory.java
<https://reviews.apache.org/r/8087/#comment29149>

    I think this will be better, because if these renames fail an error will be thrown below because the VERSION header will be incorrect. However, it seems at this point we know that the .meta file should exist since there is a .meta.tmp or .meta.old so these renames failing should be a cause to halt and inform the user?  At present the error message will be kind of confusion since if this logic is correct that error will only be seen if the file is corrupt.
- Brock Noland
On Nov. 19, 2012, 7:41 p.m., Hari Shreedharan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8087/
> -----------------------------------------------------------
>
> (Updated Nov. 19, 2012, 7:41 p.m.)
>
>
> Review request for Flume and Brock Noland.
>
>
> Description
> -------
>
> If atomic rename fails, rename the current meta file to old, rename tmp to meta and then delete old.
>
>
> This addresses bug FLUME-1699.
>     https://issues.apache.org/jira/browse/FLUME-1699
>
>
> Diffs
> -----
>
>   flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/LogFileFactory.java 82e14b0
>   flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/LogFileV3.java f768d23
>   flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Serialization.java 6b0eeb3
>   flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestLogFile.java 4b69698
>
> Diff: https://reviews.apache.org/r/8087/diff/
>
>
> Testing
> -------
>
> All unit tests pass
>
>
> Thanks,
>
> Hari Shreedharan
>
>