[GitHub] zeppelin issue #2978: [ZEPPELIN-3467] two-step, atomic configuration file wr...

2018-06-01 Thread sanjaydasgupta
Github user sanjaydasgupta commented on the issue: https://github.com/apache/zeppelin/pull/2978 There is a problem in my repo that needs correction. I am closing this PR now, and will reopen it later if possible, or return with a new PR. ---

[GitHub] zeppelin issue #2978: [ZEPPELIN-3467] two-step, atomic configuration file wr...

2018-05-31 Thread Tagar
Github user Tagar commented on the issue: https://github.com/apache/zeppelin/pull/2978 @sanjaydasgupta, `writeFile` in `FileSystemStorage` drops file first, and then renames temp file. It's not an atomic write, and also leaves a chance to loosing file that is being written.

[GitHub] zeppelin issue #2978: [ZEPPELIN-3467] two-step, atomic configuration file wr...

2018-05-31 Thread sanjaydasgupta
Github user sanjaydasgupta commented on the issue: https://github.com/apache/zeppelin/pull/2978 Hi @felixcheung, @Tagar Any suggestions on how to push this forward from here? Thanks ---

[GitHub] zeppelin issue #2978: [ZEPPELIN-3467] two-step, atomic configuration file wr...

2018-05-31 Thread zjffdu
Github user zjffdu commented on the issue: https://github.com/apache/zeppelin/pull/2978 There's nothing needs to be done in FileSystemStorage which has already use tmp file ---

[GitHub] zeppelin issue #2978: [ZEPPELIN-3467] two-step, atomic configuration file wr...

2018-05-31 Thread sanjaydasgupta
Github user sanjaydasgupta commented on the issue: https://github.com/apache/zeppelin/pull/2978 Hi @zjffdu, can you please confirm if anything more remains to be done in [FileSystemStorage.writeFile](https://github.com/apache/zeppelin/blob/master/zeppelin-zengine/src/main/java/org/apac

[GitHub] zeppelin issue #2978: [ZEPPELIN-3467] two-step, atomic configuration file wr...

2018-05-29 Thread sanjaydasgupta
Github user sanjaydasgupta commented on the issue: https://github.com/apache/zeppelin/pull/2978 Hi @zjffdu While reading the code of `FileSystemConfigStorage.java`, I noticed that the method `save(...)` delegates its work to `FileSystemStorage.writeFile(...)`. Interestingly,

[GitHub] zeppelin issue #2978: [ZEPPELIN-3467] two-step, atomic configuration file wr...

2018-05-28 Thread Tagar
Github user Tagar commented on the issue: https://github.com/apache/zeppelin/pull/2978 @sanjaydasgupta > do you recommend similar treatment for FileSystemConfigStorage also? yep, HDFS although is not a posix filesystem, but renaming a file is still implemented as an

[GitHub] zeppelin issue #2978: [ZEPPELIN-3467] two-step, atomic configuration file wr...

2018-05-27 Thread sanjaydasgupta
Github user sanjaydasgupta commented on the issue: https://github.com/apache/zeppelin/pull/2978 @felixcheung have made the suggested changes. But am not sure how to add tests for this. Any hints will be useful. @zjffdu, @Tagar do you recommend similar treatment for FileSystem

[GitHub] zeppelin issue #2978: [ZEPPELIN-3467] two-step, atomic configuration file wr...

2018-05-23 Thread zjffdu
Github user zjffdu commented on the issue: https://github.com/apache/zeppelin/pull/2978 LocalConfigStorage is the old implantation of config storage just for backward compatibility. FileSystemConfigStorage is for hadoop compatible filesystem which is new in 0.8 ---

[GitHub] zeppelin issue #2978: [ZEPPELIN-3467] two-step, atomic configuration file wr...

2018-05-23 Thread Tagar
Github user Tagar commented on the issue: https://github.com/apache/zeppelin/pull/2978 What's difference between LocalConfigStorage and FileSystemConfigStorage? I don't know Zeppelin internals that well. Wonder if this has to be fixed FileSystemConfigStorage.java as well?

[GitHub] zeppelin issue #2978: [ZEPPELIN-3467] two-step, atomic configuration file wr...

2018-05-23 Thread Tagar
Github user Tagar commented on the issue: https://github.com/apache/zeppelin/pull/2978 Thanks @sanjaydasgupta. I will give this a try today. ---

[GitHub] zeppelin issue #2978: [ZEPPELIN-3467] two-step, atomic configuration file wr...

2018-05-23 Thread sanjaydasgupta
Github user sanjaydasgupta commented on the issue: https://github.com/apache/zeppelin/pull/2978 Added code to delete the temp file if the write fails. Re-throwing the exception serves two purposes: 1) Ensure that the caller sees the same exception when the write fails, and

[GitHub] zeppelin issue #2978: [ZEPPELIN-3467] two-step, atomic configuration file wr...

2018-05-22 Thread sanjaydasgupta
Github user sanjaydasgupta commented on the issue: https://github.com/apache/zeppelin/pull/2978 Done. Also used more formal way of creating the temporary file. I've tested the integrated code on Ubuntu 16.04 - success case only :-) but am not sure how things will work on Wind