Add support writing data to hdfs by executing INSERT INTO/CTAS. #5663

wypb · 2023-07-17T04:10:15Z

Current INSERT INTO/CTAS only supprt write data to local file system, If we run the following SQL, the SQL will execute successfully, but no data can be queried from the table:

presto> create schema hive.tpch_parquet_1px with (location='hdfs://hdfsCluster/user/hive/warehouse/tpch_parquet_1px.db/');
CREATE SCHEMA
presto>
presto> create table hive.tpch_parquet_1px.lineitem with (format = 'PARQUET') as select * from tpch.sf1.lineitem;
CREATE TABLE: 6001215 rows

Query 20230714_073619_00015_8z7nw, FINISHED, 11 nodes
Splits: 301 total, 301 done (100.00%)
2:41 [6M rows, 1.97GB] [37.3K rows/s, 12.5MB/s]

presto> select * from hive.tpch_parquet_1px.lineitem limit 10;
 l_orderkey | l_partkey | l_suppkey | l_linenumber | l_quantity | l_extendedprice | l_discount | l_tax | l_returnflag | l_linestatus | l_shipdate | l_commitdate | l_receiptdate | l_shipinstruct | l_shipmode | l_comment
------------+-----------+-----------+--------------+------------+-----------------+------------+-------+--------------+--------------+------------+--------------+---------------+----------------+------------+-----------
(0 rows)

Query 20230714_073912_00016_8z7nw, FINISHED, 2 nodes
Splits: 1 total, 1 done (100.00%)
0:01 [0 rows, 0B] [0 rows/s, 0B/s]

because the data is actually written to the worker's local file system.

With this PR we can writing data to hdfs by executing INSERT INTO/CTAS.

netlify · 2023-07-17T04:10:20Z

✅ Deploy Preview for meta-velox canceled.

Name	Link
🔨 Latest commit	`abc700b`
🔍 Latest deploy log	https://app.netlify.com/sites/meta-velox/deploys/64d41aca08673d00082fd151

majetideepak · 2023-07-17T13:11:46Z

velox/dwio/common/DataSink.cpp

@@ -59,6 +61,28 @@ void WriteFileDataSink::registerLocalFileFactory() {
  DataSink::registerFactory(localWriteFileSink);
 }

+#ifdef VELOX_ENABLE_HDFS3
+std::unique_ptr<DataSink> hdfsWriteFileSink(


Can this be defined in the HDFS API?

majetideepak · 2023-07-17T13:17:56Z

@wypb We should abstract away optional checks to the storage adapter and HiveConnector initialize.
Can you refactor hdfs registration in a separate PR similar to S3 here #5469?

wypb · 2023-07-17T13:37:40Z

@majetideepak sure, let me do it in a separate pull request.

JkSelf · 2023-07-18T03:38:49Z

@majetideepak When we support the native parquet write function in Gluten, we found that Velox Parquet Writer only support write data into local file system. And we filed PR#5400 to support writing data in HDFS path when calling the velox parquet writer. Can you also have a look if you have available time? Thanks.

wypb · 2023-07-20T07:01:09Z

Hi @majetideepak Based on #5700 I refactored the code logic, could you help me review it again.

majetideepak · 2023-07-20T11:27:17Z

@JkSelf this PR seems to contain the HdfsFileSink implementation as well. Do we still need #5400?
Can you help review this PR as well? Thanks.

JkSelf · 2023-07-21T00:20:46Z

@JkSelf this PR seems to contain the HdfsFileSink implementation as well. Do we still need #5400? Can you help review this PR as well? Thanks.

Yes. It seems there is no need #5400. Thanks for your review.

JkSelf · 2023-07-24T02:52:57Z

velox/connectors/hive/storage_adapters/s3fs/CMakeLists.txt

@@ -19,7 +19,11 @@ if(VELOX_ENABLE_S3)
  target_sources(velox_s3fs PRIVATE S3FileSystem.cpp S3Util.cpp)

  target_include_directories(velox_s3fs PUBLIC ${AWSSDK_INCLUDE_DIRS})
-  target_link_libraries(velox_s3fs Folly::folly ${AWSSDK_LIBRARIES})
+  target_link_libraries(velox_s3fs Folly::folly ${AWSSDK_LIBRARIES} xsimd)


Why need s3 related changes?

@JkSelf Thank you for your reply.
Because we included velox/dwio/common/DataBuffer.h in the velox/common/file/File.h file, and DataBuffer.h depends on xsimd and gtest

Why does DataBuffer.h depend on xsimd and gtest? We need to fix this.

The CMakeLists.txt file of s3 can not be modified this time, but the CMakeLists.txt file of hdfs depends on it.
if we remove xsimd The following exception occurs at compile time:

In file included from /Users/wypb/data/code/apache/temp/velox/velox/connectors/hive/storage_adapters/hdfs/RegisterHdfsFileSystem.cpp:23: In file included from /Users/wypb/data/code/apache/temp/velox/./velox/dwio/common/DataSink.h:23: In file included from /Users/wypb/data/code/apache/temp/velox/./velox/dwio/common/DataBuffer.h:22: In file included from /Users/wypb/data/code/apache/temp/velox/./velox/buffer/Buffer.h:26: /Users/wypb/data/code/apache/temp/velox/./velox/common/base/SimdUtil.h:24:10: fatal error: 'xsimd/xsimd.hpp' file not found #include <xsimd/xsimd.hpp>

if remove gtest, The following exception occurs at compile time:

In file included from /Users/wypb/data/code/apache/temp/velox/velox/connectors/hive/storage_adapters/hdfs/RegisterHdfsFileSystem.cpp:23: In file included from /Users/wypb/data/code/apache/temp/velox/./velox/dwio/common/DataSink.h:23: In file included from /Users/wypb/data/code/apache/temp/velox/./velox/dwio/common/DataBuffer.h:22: In file included from /Users/wypb/data/code/apache/temp/velox/./velox/buffer/Buffer.h:27: In file included from /Users/wypb/data/code/apache/temp/velox/./velox/common/memory/Memory.h:38: /Users/wypb/data/code/apache/temp/velox/./velox/common/base/GTestMacros.h:24:10: fatal error: 'gtest/gtest_prod.h' file not found #include <gtest/gtest_prod.h> ^~~~~~~~~~~~~~~~~~~~ 1 warning and 1 error generated.

Including velox/common/base/GTestMacros.h inside velox/common/memory/Memory.h seems unnecessary.
I added a PR https://github.com/facebookincubator/velox/pull/5930/files to remove it. Let's see if something breaks.

wypb · 2023-07-25T10:57:31Z

@majetideepak Deepak, could you also help review this PR? thanks.

JkSelf · 2023-07-26T00:10:13Z

LGTM.

wypb · 2023-07-26T01:35:32Z

@JkSelf thank you for your review.

JkSelf · 2023-07-26T05:24:44Z

@majetideepak Deepak, do you have any further comment?

majetideepak

@wypb left some high-level comments.

majetideepak · 2023-07-26T13:04:45Z

velox/common/file/CMakeLists.txt

@@ -17,6 +17,10 @@ include_directories(.)
 add_library(velox_file File.cpp FileSystems.cpp Utils.cpp)
 target_link_libraries(velox_file velox_common_base Folly::folly)

+if(NOT VELOX_DISABLE_GOOGLETEST)


Why this change? Why does velox_file depend on gtest?

I deleted the append(dwio::common::DataBuffer<char>& buffer) api, so this dependency can be deleted.

majetideepak · 2023-07-26T13:08:06Z

velox/connectors/hive/storage_adapters/s3fs/CMakeLists.txt

@@ -19,7 +19,11 @@ if(VELOX_ENABLE_S3)
  target_sources(velox_s3fs PRIVATE S3FileSystem.cpp S3Util.cpp)

  target_include_directories(velox_s3fs PUBLIC ${AWSSDK_INCLUDE_DIRS})
-  target_link_libraries(velox_s3fs Folly::folly ${AWSSDK_LIBRARIES})
+  target_link_libraries(velox_s3fs Folly::folly ${AWSSDK_LIBRARIES} xsimd)


Why does DataBuffer.h depend on xsimd and gtest? We need to fix this.

majetideepak · 2023-07-26T13:15:49Z

velox/common/file/File.h

@@ -143,6 +144,9 @@ class WriteFile {
  // Appends data to the end of the file.
  virtual void append(std::string_view data) = 0;

+  // Appends data to the end of the file.
+  virtual void append(dwio::common::DataBuffer<char>& buffer) = 0;


Why need this new API? Can we use the existing std::string_view API on top of DataBuffer?

I tested it and found that the std::string_view API is enough to write Parquet files, thank you.

majetideepak

@wypb thanks for the fix!

majetideepak · 2023-08-07T21:49:19Z

@kgpai, @Yuhta can one of you please help with merging this change? Thanks.

wypb · 2023-08-08T15:10:13Z

hi @kgpai, @Yuhta could you please help me merging this change? Thanks.

wypb · 2023-08-09T11:53:29Z

Hi @kgpai Thanks for your review! Do you have any further comment?

facebook-github-bot · 2023-08-09T16:45:19Z

@kgpai has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

wypb · 2023-08-10T00:05:57Z

HI @kgpai I just synchronized the latest code to solve the problem of MultiFragmentTest.exchangeStatsOnFailure test failure in linux-build. could you please help me review this again. thank you.

facebook-github-bot · 2023-08-10T16:51:14Z

@kgpai has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

kgpai · 2023-08-11T14:39:19Z

@wypb This should land soon.

wypb · 2023-08-11T14:42:33Z

@kgpai Thank you.

facebook-github-bot · 2023-08-11T15:33:56Z

@kgpai merged this pull request in 81a4de5.

conbench-facebook · 2023-08-11T15:53:13Z

Conbench analyzed the 1 benchmark run on commit 81a4de5f.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

…bookincubator#5663) Summary: Current INSERT INTO/CTAS only supprt write data to local file system, If we run the following SQL, the SQL will execute successfully, but no data can be queried from the table: ``` presto> create schema hive.tpch_parquet_1px with (location='hdfs://hdfsCluster/user/hive/warehouse/tpch_parquet_1px.db/'); CREATE SCHEMA presto> presto> create table hive.tpch_parquet_1px.lineitem with (format = 'PARQUET') as select * from tpch.sf1.lineitem; CREATE TABLE: 6001215 rows Query 20230714_073619_00015_8z7nw, FINISHED, 11 nodes Splits: 301 total, 301 done (100.00%) 2:41 [6M rows, 1.97GB] [37.3K rows/s, 12.5MB/s] presto> select * from hive.tpch_parquet_1px.lineitem limit 10; l_orderkey | l_partkey | l_suppkey | l_linenumber | l_quantity | l_extendedprice | l_discount | l_tax | l_returnflag | l_linestatus | l_shipdate | l_commitdate | l_receiptdate | l_shipinstruct | l_shipmode | l_comment ------------+-----------+-----------+--------------+------------+-----------------+------------+-------+--------------+--------------+------------+--------------+---------------+----------------+------------+----------- (0 rows) Query 20230714_073912_00016_8z7nw, FINISHED, 2 nodes Splits: 1 total, 1 done (100.00%) 0:01 [0 rows, 0B] [0 rows/s, 0B/s] ``` because the data is actually written to the worker's local file system. ![image](https://github.com/facebookincubator/velox/assets/5170878/6554d670-8389-4c4e-89b2-c978fea55d14) With this PR we can writing data to hdfs by executing INSERT INTO/CTAS. Pull Request resolved: facebookincubator#5663 Reviewed By: Yuhta Differential Revision: D48195142 Pulled By: kgpai fbshipit-source-id: 1b52c59fe338a23d9006e261b3d5737534cf1efd

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 17, 2023

wypb force-pushed the insert_into_hdfs branch 6 times, most recently from 2b5f452 to 95af4ff Compare July 17, 2023 08:32

majetideepak reviewed Jul 17, 2023

View reviewed changes

wypb mentioned this pull request Jul 18, 2023

Register components for GCS, HDFS FileSystem during HiveConnector registration #5700

Closed

wypb closed this Jul 20, 2023

wypb force-pushed the insert_into_hdfs branch 2 times, most recently from 95af4ff to 40b1daf Compare July 20, 2023 01:42

wypb reopened this Jul 20, 2023

wypb force-pushed the insert_into_hdfs branch 2 times, most recently from 7bd1dc1 to c42244a Compare July 20, 2023 05:22

JkSelf mentioned this pull request Jul 21, 2023

Add Hdfs support in DataSink #5400

Closed

JkSelf reviewed Jul 24, 2023

View reviewed changes

majetideepak reviewed Jul 26, 2023

View reviewed changes

wypb force-pushed the insert_into_hdfs branch from e9c2d0f to 1fc1c6e Compare July 27, 2023 02:41

wypb force-pushed the insert_into_hdfs branch from d198ec1 to 102930f Compare August 5, 2023 09:08

majetideepak approved these changes Aug 7, 2023

View reviewed changes

majetideepak requested review from kgpai and Yuhta August 7, 2023 21:48

wypb force-pushed the insert_into_hdfs branch 6 times, most recently from e6417fd to 0286836 Compare August 9, 2023 16:14

kgpai approved these changes Aug 9, 2023

View reviewed changes

wypb force-pushed the insert_into_hdfs branch from 6a560aa to 5cd6a70 Compare August 9, 2023 22:57

Add support writing data to hdfs by executing INSERT INTO/CTAS

abc700b

wypb force-pushed the insert_into_hdfs branch from 518dad5 to abc700b Compare August 9, 2023 23:01

facebook-github-bot closed this in 81a4de5 Aug 11, 2023

facebook-github-bot added the Merged label Aug 11, 2023

wypb deleted the insert_into_hdfs branch August 14, 2023 01:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support writing data to hdfs by executing INSERT INTO/CTAS. #5663

Add support writing data to hdfs by executing INSERT INTO/CTAS. #5663

wypb commented Jul 17, 2023 •

edited

netlify bot commented Jul 17, 2023 •

edited

majetideepak Jul 17, 2023

majetideepak commented Jul 17, 2023

wypb commented Jul 17, 2023

JkSelf commented Jul 18, 2023

wypb commented Jul 20, 2023

majetideepak commented Jul 20, 2023

JkSelf commented Jul 21, 2023

JkSelf Jul 24, 2023

wypb Jul 24, 2023

majetideepak Jul 26, 2023

wypb Jul 27, 2023 •

edited

majetideepak Jul 31, 2023

wypb commented Jul 25, 2023

JkSelf commented Jul 26, 2023

wypb commented Jul 26, 2023

JkSelf commented Jul 26, 2023

majetideepak left a comment

majetideepak Jul 26, 2023

wypb Jul 27, 2023

majetideepak Jul 26, 2023

majetideepak Jul 26, 2023

wypb Jul 27, 2023

majetideepak left a comment

majetideepak commented Aug 7, 2023

wypb commented Aug 8, 2023

wypb commented Aug 9, 2023

facebook-github-bot commented Aug 9, 2023

wypb commented Aug 10, 2023

facebook-github-bot commented Aug 10, 2023

kgpai commented Aug 11, 2023

wypb commented Aug 11, 2023

facebook-github-bot commented Aug 11, 2023

conbench-facebook bot commented Aug 11, 2023

Add support writing data to hdfs by executing INSERT INTO/CTAS. #5663

Add support writing data to hdfs by executing INSERT INTO/CTAS. #5663

Conversation

wypb commented Jul 17, 2023 • edited

netlify bot commented Jul 17, 2023 • edited

✅ Deploy Preview for meta-velox canceled.

Choose a reason for hiding this comment

majetideepak commented Jul 17, 2023

wypb commented Jul 17, 2023

JkSelf commented Jul 18, 2023

wypb commented Jul 20, 2023

majetideepak commented Jul 20, 2023

JkSelf commented Jul 21, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wypb Jul 27, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wypb commented Jul 25, 2023

JkSelf commented Jul 26, 2023

wypb commented Jul 26, 2023

JkSelf commented Jul 26, 2023

majetideepak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

majetideepak left a comment

Choose a reason for hiding this comment

majetideepak commented Aug 7, 2023

wypb commented Aug 8, 2023

wypb commented Aug 9, 2023

facebook-github-bot commented Aug 9, 2023

wypb commented Aug 10, 2023

facebook-github-bot commented Aug 10, 2023

kgpai commented Aug 11, 2023

wypb commented Aug 11, 2023

facebook-github-bot commented Aug 11, 2023

conbench-facebook bot commented Aug 11, 2023

wypb commented Jul 17, 2023 •

edited

netlify bot commented Jul 17, 2023 •

edited

wypb Jul 27, 2023 •

edited