> On Feb. 5, 2015, 8:06 p.m., Himanshu Gahlaut wrote:
> > lens-server/src/main/java/org/apache/lens/server/LensServices.java, line 286
> > <https://reviews.apache.org/r/30665/diff/1/?file=850405#file850405line286>
> >
> >     Seems like the file at path will be overwritten if it already exists. 
> > Below is the code from FSDataOutputStream. We are using create without any 
> > overwrite flag and default is true. In light of this code do we still need 
> > to delete the file or am I missing something here ?
> >     ```
> >     public FSDataOutputStream create(Path f) throws IOException {
> >         return create(f, true);
> >       }
> >     
> >       /**
> >        * Create an FSDataOutputStream at the indicated Path.
> >        * @param f the file to create
> >        * @param overwrite if a file with this name already exists, then if 
> > true,
> >        *   the file will be overwritten, and if false an exception will be 
> > thrown.
> >        */
> >       public FSDataOutputStream create(Path f, boolean overwrite)
> >           throws IOException {
> >           .
> >           .
> >           .
> >     ```
> 
> Amareshwari Sriramadasu wrote:
>     Yes. create on HDFS overwrites the file. But, wanted the code in persist 
> not to have any assumptions about underlying file system implementation.
>     
>     Do you think doing a delete before create is increasing the complexity of 
> code there?
> 
> Himanshu Gahlaut wrote:
>     Increase in complexity is a bigger phrase for this context. This may fall 
> under unwanted code addition. If code is changed to use this API: public 
> FSDataOutputStream create(Path f, boolean overwrite), and pass overwrite as 
> true, then it would be coding to contract of a well documented API without 
> making any assumptions about underlying implementation.
> 
> Amareshwari Sriramadasu wrote:
>     Decision of which file system is used is at runtime. If we want to put 
> such code we should do FileSystem instanceof HDFS checks and create output 
> stream accordingly. This will introduce tight compile time dependency with 
> hadoop-hdfs module and we need to take care of compatability of hdfs api 
> across hadoop releases.
>     
>     hadoop-common api is stable and hadoop-hdfs api is unstable.
>     
>     @InterfaceAudience.Public
>     @InterfaceStability.Stable
>     public abstract class FileSystem extends Configured implements Closeable {
>     
>     ----
>     @InterfaceAudience.LimitedPrivate({ "MapReduce", "HBase" })
>     @InterfaceStability.Unstable
>     public class DistributedFileSystem extends FileSystem {
> 
> Himanshu Gahlaut wrote:
>     Didn't get this. Please help in understanding. How is using create(Path 
> f), which is current code, different from using create(Path f, boolean 
> overwrite) when both of them are in the same abstract class which is marked 
> with @InterfaceStability.Stable ?

My bad, I thought create(Path f, boolean overwrite) was only 
DistributedFileSystem api. But, as srikanth mentioned - create will not 
overwrite always, but will do so, if the lease holder is same - let me check if 
that is true, even if the flag is true.


- Amareshwari


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


On Feb. 6, 2015, 6:01 a.m., Amareshwari Sriramadasu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30665/
> -----------------------------------------------------------
> 
> (Updated Feb. 6, 2015, 6:01 a.m.)
> 
> 
> Review request for lens, Himanshu Gahlaut and Srikanth Sundarrajan.
> 
> 
> Bugs: LENS-278
>     https://issues.apache.org/jira/browse/LENS-278
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> Main issue is that rename is a no-op in HDFS when destination exists
> 
> 
> Diffs
> -----
> 
>   lens-server/src/main/java/org/apache/lens/server/LensServices.java b1f087c 
> 
> Diff: https://reviews.apache.org/r/30665/diff/
> 
> 
> Testing
> -------
> 
> Will test on docker with hdfs and udpate
> 
> 
> [INFO] 
> ------------------------------------------------------------------------
> [INFO] Reactor Summary:
> [INFO] 
> [INFO] Lens Checkstyle Rules ............................. SUCCESS [2.084s]
> [INFO] Lens .............................................. SUCCESS [1.761s]
> [INFO] Lens API .......................................... SUCCESS [5.678s]
> [INFO] Lens API for server and extensions ................ SUCCESS [5.726s]
> [INFO] Lens Cube ......................................... SUCCESS [6:28.915s]
> [INFO] Lens DB storage ................................... SUCCESS [10.423s]
> [INFO] Lens Query Library ................................ SUCCESS [4.603s]
> [INFO] Lens Hive Driver .................................. SUCCESS [2:39.451s]
> [INFO] Lens Driver for JDBC .............................. SUCCESS [25.482s]
> [INFO] Lens Server ....................................... SUCCESS [4:34.883s]
> [INFO] Lens client ....................................... SUCCESS [19.892s]
> [INFO] Lens CLI .......................................... SUCCESS [1:33.574s]
> [INFO] Lens Examples ..................................... SUCCESS [0.816s]
> [INFO] Lens Distribution ................................. SUCCESS [3.792s]
> [INFO] Lens Client Distribution .......................... SUCCESS [5.748s]
> [INFO] Lens ML Lib ....................................... SUCCESS [43.244s]
> [INFO] Lens Regression ................................... SUCCESS [0.841s]
> [INFO] 
> ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] 
> ------------------------------------------------------------------------
> [INFO] Total time: 17:27.795s
> [INFO] Finished at: Thu Feb 05 13:02:53 UTC 2015
> [INFO] Final Memory: 99M/980M
> [INFO] 
> ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Amareshwari Sriramadasu
> 
>

Reply via email to