Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/17121 )
Change subject: IMPALA-7712: Support Google Cloud Storage ...................................................................... Patch Set 10: (1 comment) > Patch Set 10: Code-Review+2 > > (1 comment) > > Given the analysis in IMPALA-10563, it seems fine to disable those test cases > for now. > > See my note about IMPALA-10579. I think it is ok to include this partial fix, > as it seems better than what we have right now. If IMPALA-10579 was landing > very soon, I would be ok with removing this piece of the fix and relying on > IMPALA-10579. > > This change makes sense to me, and it is good to have the GCS support land. Thanks Joe's review! IMPALA-10579 (https://gerrit.cloudera.org/c/17171/) will take some time to land. So let's have the conservative fix for GCS first. http://gerrit.cloudera.org:8080/#/c/17121/10/fe/src/main/java/org/apache/impala/common/FileSystemUtil.java File fe/src/main/java/org/apache/impala/common/FileSystemUtil.java: http://gerrit.cloudera.org:8080/#/c/17121/10/fe/src/main/java/org/apache/impala/common/FileSystemUtil.java@713 PS10, Line 713: /** : * Wrapper around FileSystem.listStatusIterator() to make sure the path exists. : * : * @throws FileNotFoundException if <code>p</code> does not exist : * @throws IOException if any I/O error occurredd : */ : public static RemoteIterator<FileStatus> listStatusIterator(FileSystem fs, Path p) : throws IOException { : RemoteIterator<FileStatus> iterator = fs.listStatusIterator(p); : // Some FileSystem implementations like GoogleHadoopFileSystem doesn't check : // existence of the start path when creating the RemoteIterator. Instead, their : // iterators throw the FileNotFoundException in the first call of hasNext() when : // the start path doesn't exist. Here we call hasNext() to ensure start path exists. : iterator.hasNext(); : return iterator; > This code will be replaced by IMPALA-10579. Yeah, exactly! For IMPALA-10579 (https://gerrit.cloudera.org/c/17171/), I plan to test the patch on Ozone, S3 and ABFS so it will take some time. The changes in this patch is conservative so we can assure it won't impact other filesystems. (I have verified it on HDFS and GCS) -- To view, visit http://gerrit.cloudera.org:8080/17121 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia91ec956de3b620cccf6a1244b56b7da7a45b32b Gerrit-Change-Number: 17121 Gerrit-PatchSet: 10 Gerrit-Owner: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com> Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Comment-Date: Fri, 12 Mar 2021 23:17:16 +0000 Gerrit-HasComments: Yes
