From 84ef2977ffafa6b4d4a36abfcc2083d100548a7a Mon Sep 17 00:00:00 2001 From: shendanfeng01 Date: Thu, 2 Feb 2023 11:43:29 +0800 Subject: [PATCH 01/10] fix 1016 --- .../server/optimize/SupportHiveCommit.java | 4 ++++ .../netease/arctic/io/ArcticHadoopFileIO.java | 21 +++++++++++++++++-- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/ams/ams-server/src/main/java/com/netease/arctic/ams/server/optimize/SupportHiveCommit.java b/ams/ams-server/src/main/java/com/netease/arctic/ams/server/optimize/SupportHiveCommit.java index fd307a4241..45c2cfa7ba 100644 --- a/ams/ams-server/src/main/java/com/netease/arctic/ams/server/optimize/SupportHiveCommit.java +++ b/ams/ams-server/src/main/java/com/netease/arctic/ams/server/optimize/SupportHiveCommit.java @@ -149,6 +149,10 @@ private DataFile moveTargetFiles(DataFile targetFile, String hiveLocation) { String newFilePath = TableFileUtils.getNewFilePath(hiveLocation, oldFilePath); if (!arcticTable.io().exists(newFilePath)) { + if (!arcticTable.io().exists(hiveLocation)) { + LOG.debug("{} hive location {} does not exist and need to mkdir before rename", arcticTable.id(), hiveLocation); + arcticTable.io().mkdirs(hiveLocation); + } arcticTable.io().rename(oldFilePath, newFilePath); LOG.debug("{} move file from {} to {}", arcticTable.id(), oldFilePath, newFilePath); } diff --git a/core/src/main/java/com/netease/arctic/io/ArcticHadoopFileIO.java b/core/src/main/java/com/netease/arctic/io/ArcticHadoopFileIO.java index 464a2d7b3b..9da0313543 100644 --- a/core/src/main/java/com/netease/arctic/io/ArcticHadoopFileIO.java +++ b/core/src/main/java/com/netease/arctic/io/ArcticHadoopFileIO.java @@ -31,6 +31,8 @@ import org.apache.iceberg.relocated.com.google.common.annotations.VisibleForTesting; import org.apache.iceberg.relocated.com.google.common.base.Preconditions; import org.apache.iceberg.relocated.com.google.common.collect.Lists; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.io.IOException; import java.io.UncheckedIOException; @@ -41,6 +43,8 @@ * Implementation of {@link ArcticFileIO} for hadoop file system with authentication. */ public class ArcticHadoopFileIO extends HadoopFileIO implements ArcticFileIO { + private static final Logger LOG = LoggerFactory.getLogger(ArcticHadoopFileIO.class); + private final TableMetaStore tableMetaStore; public ArcticHadoopFileIO(TableMetaStore tableMetaStore) { @@ -84,6 +88,9 @@ public boolean deleteFileWithResult(String path, boolean recursive) { } catch (IOException e) { result = false; } + if (result == false) { + LOG.warn("File to delete file " + path + " and result is false, need to check the hdfs path"); + } return result; }); } @@ -165,7 +172,12 @@ public boolean rename(String src, String dts) { Path dtsPath = new Path(dts); FileSystem fs = getFs(srcPath); try { - return fs.rename(srcPath, dtsPath); + if (fs.rename(srcPath, dtsPath) == false) { + throw new IOException("Failed to rename: from " + src + " to " + dts + + " and result is false, need to check the hdfs path"); + } else { + return true; + } } catch (IOException e) { throw new UncheckedIOException("Failed to rename: from " + src + " to " + dts, e); } @@ -196,7 +208,12 @@ public boolean mkdirs(String path) { Path filePath = new Path(path); FileSystem fs = getFs(filePath); try { - return fs.mkdirs(filePath); + if (fs.mkdirs(filePath) == false) { + throw new IOException("Failed to mkdirs: path " + path + + " and result is false, need to check the hdfs path"); + } else { + return true; + } } catch (IOException e) { throw new UncheckedIOException("Failed to mkdirs: path " + path, e); } From 4eeeabcb33dbf3545b4d10183f6513437b5c2754 Mon Sep 17 00:00:00 2001 From: shendanfeng01 Date: Thu, 2 Feb 2023 14:04:46 +0800 Subject: [PATCH 02/10] fix grammer --- .../netease/arctic/io/ArcticHadoopFileIO.java | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/core/src/main/java/com/netease/arctic/io/ArcticHadoopFileIO.java b/core/src/main/java/com/netease/arctic/io/ArcticHadoopFileIO.java index 9da0313543..73685b1149 100644 --- a/core/src/main/java/com/netease/arctic/io/ArcticHadoopFileIO.java +++ b/core/src/main/java/com/netease/arctic/io/ArcticHadoopFileIO.java @@ -71,7 +71,7 @@ public void deleteFile(String path) { try { fs.delete(toDelete, false); } catch (IOException e) { - throw new UncheckedIOException("Failed to delete file: " + path, e); + throw new UncheckedIOException("Fail to delete file: " + path, e); } return null; }); @@ -89,7 +89,7 @@ public boolean deleteFileWithResult(String path, boolean recursive) { result = false; } if (result == false) { - LOG.warn("File to delete file " + path + " and result is false, need to check the hdfs path"); + LOG.warn("Fail to delete file " + path + " and file system return false, false, need to check the hdfs path"); } return result; }); @@ -173,13 +173,13 @@ public boolean rename(String src, String dts) { FileSystem fs = getFs(srcPath); try { if (fs.rename(srcPath, dtsPath) == false) { - throw new IOException("Failed to rename: from " + src + " to " + dts + - " and result is false, need to check the hdfs path"); + throw new IOException("Fail to rename: from " + src + " to " + dts + + " and file system return false, need to check the hdfs path"); } else { return true; } } catch (IOException e) { - throw new UncheckedIOException("Failed to rename: from " + src + " to " + dts, e); + throw new UncheckedIOException("Fail to rename: from " + src + " to " + dts, e); } }); } @@ -197,7 +197,7 @@ public boolean exists(String path) { try { return fs.exists(filePath); } catch (IOException e) { - throw new UncheckedIOException("Failed to check file exist for " + path, e); + throw new UncheckedIOException("Fail to check file exist for " + path, e); } }); } @@ -209,13 +209,13 @@ public boolean mkdirs(String path) { FileSystem fs = getFs(filePath); try { if (fs.mkdirs(filePath) == false) { - throw new IOException("Failed to mkdirs: path " + path + - " and result is false, need to check the hdfs path"); + throw new IOException("Fail to mkdirs: path " + path + + " and file system return false,, need to check the hdfs path"); } else { return true; } } catch (IOException e) { - throw new UncheckedIOException("Failed to mkdirs: path " + path, e); + throw new UncheckedIOException("Fail to mkdirs: path " + path, e); } }); } From 9ae24d31e805860c3623b7eac4f5dde817c43a9a Mon Sep 17 00:00:00 2001 From: shendanfeng01 Date: Thu, 2 Feb 2023 14:06:33 +0800 Subject: [PATCH 03/10] fix type mistake --- .../src/main/java/com/netease/arctic/io/ArcticHadoopFileIO.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/com/netease/arctic/io/ArcticHadoopFileIO.java b/core/src/main/java/com/netease/arctic/io/ArcticHadoopFileIO.java index 73685b1149..9c1719e5ac 100644 --- a/core/src/main/java/com/netease/arctic/io/ArcticHadoopFileIO.java +++ b/core/src/main/java/com/netease/arctic/io/ArcticHadoopFileIO.java @@ -89,7 +89,7 @@ public boolean deleteFileWithResult(String path, boolean recursive) { result = false; } if (result == false) { - LOG.warn("Fail to delete file " + path + " and file system return false, false, need to check the hdfs path"); + LOG.warn("Fail to delete file " + path + " and file system return false, need to check the hdfs path"); } return result; }); From 5d124d3162070f75f1dd10839f4d98b4a4e36af0 Mon Sep 17 00:00:00 2001 From: shendanfeng01 Date: Mon, 6 Feb 2023 10:02:38 +0800 Subject: [PATCH 04/10] fix-1016 --- .../main/java/com/netease/arctic/hive/CachedHiveClientPool.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hive/src/main/java/com/netease/arctic/hive/CachedHiveClientPool.java b/hive/src/main/java/com/netease/arctic/hive/CachedHiveClientPool.java index bfd8322842..cc8a9130f3 100644 --- a/hive/src/main/java/com/netease/arctic/hive/CachedHiveClientPool.java +++ b/hive/src/main/java/com/netease/arctic/hive/CachedHiveClientPool.java @@ -73,7 +73,7 @@ private synchronized void init() { @Override public R run(Action action) throws TException, InterruptedException { try { - return tableMetaStore.doAs(() -> clientPool().run(action)); + return tableMetaStore.doAs(() -> clientPool().run(action, true)); } catch (RuntimeException e) { throw throwTException(e); } From 5e590215e9f7256dcd0a2d6fa0cc3429224ddab4 Mon Sep 17 00:00:00 2001 From: shendanfeng01 Date: Tue, 7 Feb 2023 20:11:19 +0800 Subject: [PATCH 05/10] refactoring functions and removing useless code --- .../java/com/netease/arctic/io/ArcticFileIO.java | 7 +++---- .../com/netease/arctic/io/ArcticHadoopFileIO.java | 14 ++++++-------- .../netease/arctic/hive/CachedHiveClientPool.java | 2 +- 3 files changed, 10 insertions(+), 13 deletions(-) diff --git a/core/src/main/java/com/netease/arctic/io/ArcticFileIO.java b/core/src/main/java/com/netease/arctic/io/ArcticFileIO.java index 4a1486c261..963471b1fa 100644 --- a/core/src/main/java/com/netease/arctic/io/ArcticFileIO.java +++ b/core/src/main/java/com/netease/arctic/io/ArcticFileIO.java @@ -42,6 +42,7 @@ public interface ArcticFileIO extends FileIO { * Check if a path exists. * * @param path source pathmkdir + * @return true if the path exists; */ boolean exists(String path); @@ -49,18 +50,16 @@ public interface ArcticFileIO extends FileIO { * Create a new directory. * * @param path source path - * @return true if the create success; */ - boolean mkdirs(String path); + void mkdirs(String path); /** * Rename file from old path to new path * * @param oldpath source path * @param newPath target path - * @return true if the rename success; */ - boolean rename(String oldpath, String newPath); + void rename(String oldpath, String newPath); /** Delete a file. * diff --git a/core/src/main/java/com/netease/arctic/io/ArcticHadoopFileIO.java b/core/src/main/java/com/netease/arctic/io/ArcticHadoopFileIO.java index 9c1719e5ac..0368d7a8a4 100644 --- a/core/src/main/java/com/netease/arctic/io/ArcticHadoopFileIO.java +++ b/core/src/main/java/com/netease/arctic/io/ArcticHadoopFileIO.java @@ -166,8 +166,8 @@ public boolean isEmptyDirectory(String location) { } @Override - public boolean rename(String src, String dts) { - return tableMetaStore.doAs(() -> { + public void rename(String src, String dts) { + tableMetaStore.doAs(() -> { Path srcPath = new Path(src); Path dtsPath = new Path(dts); FileSystem fs = getFs(srcPath); @@ -175,12 +175,11 @@ public boolean rename(String src, String dts) { if (fs.rename(srcPath, dtsPath) == false) { throw new IOException("Fail to rename: from " + src + " to " + dts + " and file system return false, need to check the hdfs path"); - } else { - return true; } } catch (IOException e) { throw new UncheckedIOException("Fail to rename: from " + src + " to " + dts, e); } + return null; }); } @@ -203,20 +202,19 @@ public boolean exists(String path) { } @Override - public boolean mkdirs(String path) { - return tableMetaStore.doAs(() -> { + public void mkdirs(String path) { + tableMetaStore.doAs(() -> { Path filePath = new Path(path); FileSystem fs = getFs(filePath); try { if (fs.mkdirs(filePath) == false) { throw new IOException("Fail to mkdirs: path " + path + " and file system return false,, need to check the hdfs path"); - } else { - return true; } } catch (IOException e) { throw new UncheckedIOException("Fail to mkdirs: path " + path, e); } + return null; }); } diff --git a/hive/src/main/java/com/netease/arctic/hive/CachedHiveClientPool.java b/hive/src/main/java/com/netease/arctic/hive/CachedHiveClientPool.java index cc8a9130f3..bfd8322842 100644 --- a/hive/src/main/java/com/netease/arctic/hive/CachedHiveClientPool.java +++ b/hive/src/main/java/com/netease/arctic/hive/CachedHiveClientPool.java @@ -73,7 +73,7 @@ private synchronized void init() { @Override public R run(Action action) throws TException, InterruptedException { try { - return tableMetaStore.doAs(() -> clientPool().run(action, true)); + return tableMetaStore.doAs(() -> clientPool().run(action)); } catch (RuntimeException e) { throw throwTException(e); } From 632a710c2b0ced378ef513cbc9cd10f50ab0db8b Mon Sep 17 00:00:00 2001 From: shendanfeng01 Date: Tue, 7 Feb 2023 20:15:58 +0800 Subject: [PATCH 06/10] fix compilation --- .../java/com/netease/arctic/io/ArcticFileIoDummy.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/core/src/test/java/com/netease/arctic/io/ArcticFileIoDummy.java b/core/src/test/java/com/netease/arctic/io/ArcticFileIoDummy.java index a0c80f66dd..921a008a1c 100644 --- a/core/src/test/java/com/netease/arctic/io/ArcticFileIoDummy.java +++ b/core/src/test/java/com/netease/arctic/io/ArcticFileIoDummy.java @@ -49,13 +49,13 @@ public boolean exists(String path) { } @Override - public boolean mkdirs(String path) { - return false; + public void mkdirs(String path) { + } @Override - public boolean rename(String oldpath, String newPath) { - return false; + public void rename(String oldpath, String newPath) { + } @Override From 29d8a985014efe3bce7a9930778b9a4d56cb039b Mon Sep 17 00:00:00 2001 From: wangtaohz <103108928+wangtaohz@users.noreply.github.com> Date: Wed, 8 Feb 2023 14:57:35 +0800 Subject: [PATCH 07/10] Apply suggestions from code review Co-authored-by: ZhouJinsong --- .../main/java/com/netease/arctic/io/ArcticHadoopFileIO.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/com/netease/arctic/io/ArcticHadoopFileIO.java b/core/src/main/java/com/netease/arctic/io/ArcticHadoopFileIO.java index 0368d7a8a4..a4378b6bbb 100644 --- a/core/src/main/java/com/netease/arctic/io/ArcticHadoopFileIO.java +++ b/core/src/main/java/com/netease/arctic/io/ArcticHadoopFileIO.java @@ -88,7 +88,7 @@ public boolean deleteFileWithResult(String path, boolean recursive) { } catch (IOException e) { result = false; } - if (result == false) { + if (!result ) { LOG.warn("Fail to delete file " + path + " and file system return false, need to check the hdfs path"); } return result; @@ -172,7 +172,7 @@ public void rename(String src, String dts) { Path dtsPath = new Path(dts); FileSystem fs = getFs(srcPath); try { - if (fs.rename(srcPath, dtsPath) == false) { + if (!fs.rename(srcPath, dtsPath)) { throw new IOException("Fail to rename: from " + src + " to " + dts + " and file system return false, need to check the hdfs path"); } @@ -207,7 +207,7 @@ public void mkdirs(String path) { Path filePath = new Path(path); FileSystem fs = getFs(filePath); try { - if (fs.mkdirs(filePath) == false) { + if (!fs.mkdirs(filePath)) { throw new IOException("Fail to mkdirs: path " + path + " and file system return false,, need to check the hdfs path"); } From b789b86b995147575157fb781301832be11cee7c Mon Sep 17 00:00:00 2001 From: wangtaohz <103108928+wangtaohz@users.noreply.github.com> Date: Wed, 8 Feb 2023 15:02:11 +0800 Subject: [PATCH 08/10] fix checkstyle --- .../main/java/com/netease/arctic/io/ArcticHadoopFileIO.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/com/netease/arctic/io/ArcticHadoopFileIO.java b/core/src/main/java/com/netease/arctic/io/ArcticHadoopFileIO.java index a4378b6bbb..6db6d354b4 100644 --- a/core/src/main/java/com/netease/arctic/io/ArcticHadoopFileIO.java +++ b/core/src/main/java/com/netease/arctic/io/ArcticHadoopFileIO.java @@ -78,7 +78,8 @@ public void deleteFile(String path) { } @Override - public boolean deleteFileWithResult(String path, boolean recursive) { + public boolean deleteFileWith + (String path, boolean recursive) { return tableMetaStore.doAs(() -> { Path toDelete = new Path(path); FileSystem fs = getFs(toDelete); @@ -88,7 +89,7 @@ public boolean deleteFileWithResult(String path, boolean recursive) { } catch (IOException e) { result = false; } - if (!result ) { + if (!result) { LOG.warn("Fail to delete file " + path + " and file system return false, need to check the hdfs path"); } return result; From 54f3516cbc92eb98a828ad274ca76af539d292b5 Mon Sep 17 00:00:00 2001 From: wangtaohz <103108928+wangtaohz@users.noreply.github.com> Date: Wed, 8 Feb 2023 15:09:08 +0800 Subject: [PATCH 09/10] fix checkstyle --- .../main/java/com/netease/arctic/io/ArcticHadoopFileIO.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/core/src/main/java/com/netease/arctic/io/ArcticHadoopFileIO.java b/core/src/main/java/com/netease/arctic/io/ArcticHadoopFileIO.java index 6db6d354b4..b38af68dcb 100644 --- a/core/src/main/java/com/netease/arctic/io/ArcticHadoopFileIO.java +++ b/core/src/main/java/com/netease/arctic/io/ArcticHadoopFileIO.java @@ -78,8 +78,7 @@ public void deleteFile(String path) { } @Override - public boolean deleteFileWith - (String path, boolean recursive) { + public boolean deleteFileWith(String path, boolean recursive) { return tableMetaStore.doAs(() -> { Path toDelete = new Path(path); FileSystem fs = getFs(toDelete); From 61a9e1f5d2dcba0031bf3b34fcf82f7adfa6b66c Mon Sep 17 00:00:00 2001 From: wangtaohz <103108928+wangtaohz@users.noreply.github.com> Date: Wed, 8 Feb 2023 15:16:09 +0800 Subject: [PATCH 10/10] fix checkstyle --- .../src/main/java/com/netease/arctic/io/ArcticHadoopFileIO.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/com/netease/arctic/io/ArcticHadoopFileIO.java b/core/src/main/java/com/netease/arctic/io/ArcticHadoopFileIO.java index b38af68dcb..b2c2f47740 100644 --- a/core/src/main/java/com/netease/arctic/io/ArcticHadoopFileIO.java +++ b/core/src/main/java/com/netease/arctic/io/ArcticHadoopFileIO.java @@ -78,7 +78,7 @@ public void deleteFile(String path) { } @Override - public boolean deleteFileWith(String path, boolean recursive) { + public boolean deleteFileWithResult(String path, boolean recursive) { return tableMetaStore.doAs(() -> { Path toDelete = new Path(path); FileSystem fs = getFs(toDelete);