Skip to content

ARROW-2034: [C++] Filesystem implementation for Azure Blob Storage #12914

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
shefali163 wants to merge 38 commits into from
Closed

ARROW-2034: [C++] Filesystem implementation for Azure Blob Storage #12914

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
b60c7f4
ARROW-2034: [C++] Filesystem implementation for AzureBlobFileSystem
Apr 18, 2022
1e2d0a3
ARROW-2034: [C++] Fixed formatting issues
Apr 21, 2022
d3cffa2
ARROW-2034: [C++] Fixed formatting issues
Apr 21, 2022
af13444
Added -DARROW_AZURE in ci
May 1, 2022
1026e15
Added CXX_STANDARD and CXX_STANDARD_REQUIRED
May 9, 2022
5f8b82a
Added mocked test file
May 26, 2022
b53a834
Merge remote-tracking branch 'upstream/master' into ARROW-2034-azurefs
May 26, 2022
5bd8210
Turned -DARROW_AZURE=OFF in appveyor-cpp-build
May 26, 2022
eead673
Changed default C++ version
Jun 1, 2022
f99fad5
Changed LibXml2 target
Jun 1, 2022
e2008d8
Fixing CMake styling issues
Jun 2, 2022
bb49f62
Enabling ARROW_AZURE flag
Jun 3, 2022
323b394
Added OpenSSL dependency
Jun 6, 2022
95cc602
Disabling ARROW_AZURE in windows-mingw
Jun 15, 2022
9350b4c
Fixing lint issues
Jun 15, 2022
9cd1a1a
Fixing azurefs_test
Jun 15, 2022
8ba75ae
Added Azurite
Jun 25, 2022
ca9a6fc
Added azurefs_objlib
Jun 25, 2022
f067ba9
Reverting azure object library changes
Jun 26, 2022
1f26725
Added permissions to install_azurite.sh
Jun 27, 2022
c16f853
chmod +x ci/scripts/install_azurite.sh
kou Jun 28, 2022
14267c2
Don't specify CMAKE_CXX_STANDARD by default
kou Jun 28, 2022
11ce11f
Fix system detection
kou Jun 28, 2022
a428a2b
Fix syntax
kou Jun 28, 2022
a62d104
Fix style
kou Jun 28, 2022
488e223
Fix style
kou Jun 28, 2022
3831a88
Running azurite through boost::process
Jun 28, 2022
8248c48
Fixed naming in azurefs_test.cc
Jul 14, 2022
dcd6e30
Fixed naming in azurefs.cc
Jul 19, 2022
b15a6b1
Fixed OpenOutputStream
Jul 26, 2022
a06c480
Merge remote-tracking branch 'upstream/master' into ARROW-2034-azurefs
Jul 26, 2022
a40a316
Added uri.Parse()
Aug 3, 2022
8600b6b
Updated versions.txt
Aug 28, 2022
18dc625
Merge remote-tracking branch 'upstream/master' into ARROW-2034-azurefs
Aug 28, 2022
b532701
Fixed ARROW_AZURE_STORAGE_BLOBS_URL
Aug 29, 2022
200592b
Added libxml2-dev
Aug 29, 2022
fe5b311
Merge remote-tracking branch 'upstream/master' into ARROW-2034-azurefs
Oct 2, 2022
3ea2d7f
Fixed build errors
Oct 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions ci/appveyor-cpp-build.bat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ cmake -G "%GENERATOR%" %CMAKE_ARGS% ^
-DARROW_PARQUET=ON ^
-DARROW_PYTHON=ON ^
-DARROW_S3=%ARROW_S3% ^
-DARROW_AZURE=OFF ^
Copy link
Copy Markdown
Member

@kou kou May 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you keep this list in alphabetical order?

-DARROW_SUBSTRAIT=ON ^
-DARROW_VERBOSE_THIRDPARTY_BUILD=OFF ^
-DARROW_WITH_BROTLI=ON ^
Expand Down
1 change: 1 addition & 0 deletions ci/scripts/cpp_build.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ mkdir -p ${build_dir}
pushd ${build_dir}

cmake \
-DARROW_AZURE=${ARROW_AZURE:-OFF} \
-DARROW_BOOST_USE_SHARED=${ARROW_BOOST_USE_SHARED:-ON} \
-DARROW_BUILD_BENCHMARKS_REFERENCE=${ARROW_BUILD_BENCHMARKS:-OFF} \
-DARROW_BUILD_BENCHMARKS=${ARROW_BUILD_BENCHMARKS:-OFF} \
Expand Down
5 changes: 5 additions & 0 deletions cpp/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -816,6 +816,11 @@ if(ARROW_WITH_OPENTELEMETRY)
opentelemetry-cpp::otlp_http_exporter)
endif()

if (ARROW_AZURE)
list(APPEND ARROW_LINK_LIBS ${AZURESDK_LINK_LIBRARIES})
list(APPEND ARROW_STATIC_LINK_LIBS ${AZURESDK_LINK_LIBRARIES})
endif()

if(ARROW_WITH_UTF8PROC)
list(APPEND ARROW_LINK_LIBS utf8proc::utf8proc)
list(APPEND ARROW_STATIC_LINK_LIBS utf8proc::utf8proc)
Expand Down
2 changes: 2 additions & 0 deletions cpp/cmake_modules/DefineOptions.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,8 @@ if("${CMAKE_SOURCE_DIR}" STREQUAL "${CMAKE_CURRENT_SOURCE_DIR}")
#----------------------------------------------------------------------
set_option_category("Project component")

define_option(ARROW_AZURE "Build Arrow with Azure support (requires the Azure SDK for C++)" OFF)

define_option(ARROW_BUILD_UTILITIES "Build Arrow commandline utilities" OFF)

define_option(ARROW_COMPUTE "Build the Arrow Compute Modules" OFF)
Expand Down
139 changes: 139 additions & 0 deletions cpp/cmake_modules/ThirdpartyToolchain.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,46 @@ else()
"${THIRDPARTY_MIRROR_URL}/aws-sdk-cpp-${ARROW_AWSSDK_BUILD_VERSION}.tar.gz")
endif()

if(DEFINED ENV{ARROW_AZURE_CORE_URL})
set(AZURE_CORE_SOURCE_URL "$ENV{ARROW_AZURE_CORE_URL}")
else()
set_urls(AZURE_CORE_SOURCE_URL
"https://github.com/Azure/azure-sdk-for-cpp/archive/azure-core_${ARROW_AZURE_CORE_BUILD_VERSION}.tar.gz"
)
endif()

if(DEFINED ENV{ARROW_AZURE_IDENTITY_URL})
set(AZURE_IDENTITY_SOURCE_URL "$ENV{ARROW_AZURE_IDENTITY_URL}")
else()
set_urls(AZURE_IDENTITY_SOURCE_URL
"https://github.com/Azure/azure-sdk-for-cpp/archive/azure-identity_${ARROW_AZURE_IDENTITY_BUILD_VERSION}.tar.gz"
)
endif()

if(DEFINED ENV{ARROW_AZURE_STORAGE_BLOB_URL})
set(AZURE_STORAGE_BLOB_SOURCE_URL "$ENV{ARROW_AZURE_STORAGE_BLOB_URL}")
else()
set_urls(AZURE_STORAGE_BLOB_SOURCE_URL
"https://github.com/Azure/azure-sdk-for-cpp/archive/azure-storage-blobs_${ARROW_AZURE_STORAGE_BLOB_BUILD_VERSION}.tar.gz"
)
endif()

if(DEFINED ENV{ARROW_AZURE_STORAGE_COMMON_URL})
set(AZURE_STORAGE_COMMON_SOURCE_URL "$ENV{ARROW_AZURE_STORAGE_COMMON_URL}")
else()
set_urls(AZURE_STORAGE_COMMON_SOURCE_URL
"https://github.com/Azure/azure-sdk-for-cpp/archive/azure-storage-common_${ARROW_AZURE_STORAGE_COMMON_BUILD_VERSION}.tar.gz"
)
endif()

if(DEFINED ENV{ARROW_AZURE_STORAGE_FILES_DATALAKE_URL})
set(AZURE_STORAGE_FILES_DATALAKE_SOURCE_URL "$ENV{ARROW_AZURE_STORAGE_FILES_DATALAKE_URL}")
else()
set_urls(AZURE_STORAGE_FILES_DATALAKE_SOURCE_URL
"https://github.com/Azure/azure-sdk-for-cpp/archive/azure-storage-files-datalake_${ARROW_AZURE_STORAGE_FILES_DATALAKE_BUILD_VERSION}.tar.gz"
)
endif()

if(DEFINED ENV{ARROW_BOOST_URL})
set(BOOST_SOURCE_URL "$ENV{ARROW_BOOST_URL}")
else()
Expand Down Expand Up @@ -4553,6 +4593,105 @@ if(ARROW_S3)
endif()
endif()

macro(build_azuresdk)
Copy link
Copy Markdown
Member

@kou kou Apr 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you build Azure C++ SDK by externalproject_add in this?

message(STATUS "Building Azure C++ SDK from source")

set(AZURESDK_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/azuresdk_ep-install")
set(AZURESDK_INCLUDE_DIR "${AZURESDK_PREFIX}/include")

set(AZURESDK_CMAKE_ARGS
${EP_COMMON_CMAKE_ARGS}
-DBUILD_TESTING=OFF
-DCMAKE_INSTALL_LIBDIR=lib
"-DCMAKE_INSTALL_PREFIX=${AZURESDK_PREFIX}"
-DCMAKE_PREFIX_PATH=${AZURESDK_PREFIX})

file(MAKE_DIRECTORY ${AZURESDK_INCLUDE_DIR})

# Azure C++ SDK related libraries to link statically
set(_AZURESDK_LIBS
azure-core
azure-identity
azure-storage-blobs
azure-storage-common
azure-storage-files-datalake)
set(AZURESDK_LIBRARIES)
set(AZURESDK_LIBRARIES_CPP)
foreach(_AZURESDK_LIB ${_AZURESDK_LIBS})
string(TOUPPER ${_AZURESDK_LIB} _AZURESDK_LIB_UPPER)
string(REPLACE "-" "_" _AZURESDK_LIB_NAME_PREFIX ${_AZURESDK_LIB_UPPER})
list(APPEND AZURESDK_LIBRARIES_CPP "${_AZURESDK_LIB}-cpp")
set(_AZURESDK_TARGET_NAME Azure::${_AZURESDK_LIB})
set(_AZURESDK_STATIC_LIBRARY
"${AZURESDK_PREFIX}/lib/${CMAKE_STATIC_LIBRARY_PREFIX}${_AZURESDK_LIB}${CMAKE_STATIC_LIBRARY_SUFFIX}"
)
add_library(${_AZURESDK_TARGET_NAME} STATIC IMPORTED)
set_target_properties(${_AZURESDK_TARGET_NAME}
PROPERTIES IMPORTED_LOCATION ${_AZURESDK_STATIC_LIBRARY}
INTERFACE_INCLUDE_DIRECTORIES
"${AZURESDK_INCLUDE_DIR}")
set("${_AZURESDK_LIB_NAME_PREFIX}_STATIC_LIBRARY" ${_AZURESDK_STATIC_LIBRARY})
list(APPEND AZURESDK_LIBRARIES ${_AZURESDK_TARGET_NAME})
endforeach()

externalproject_add(azure_core_ep
${EP_LOG_OPTIONS}
URL ${AZURE_CORE_SOURCE_URL}
URL_HASH "SHA256=${ARROW_AZURE_CORE_BUILD_SHA256_CHECKSUM}"
CMAKE_ARGS ${AZURESDK_CMAKE_ARGS}
BUILD_BYPRODUCTS ${AZURE_CORE_STATIC_LIBRARY})
add_dependencies(Azure::azure-core azure_core_ep)

externalproject_add(azure_identity_ep
${EP_LOG_OPTIONS}
URL ${AZURE_IDENTITY_SOURCE_URL}
URL_HASH "SHA256=${ARROW_AZURE_IDENTITY_BUILD_SHA256_CHECKSUM}"
CMAKE_ARGS ${AZURESDK_CMAKE_ARGS}
BUILD_BYPRODUCTS ${AZURE_IDENTITY_STATIC_LIBRARY})
add_dependencies(Azure::azure-identity azure_identity_ep)

externalproject_add(azure_storage_blobs_ep
${EP_LOG_OPTIONS}
URL ${AZURE_STORAGE_BLOB_SOURCE_URL}
URL_HASH "SHA256=${ARROW_AZURE_STORAGE_BLOB_BUILD_SHA256_CHECKSUM}"
CMAKE_ARGS ${AZURESDK_CMAKE_ARGS}
BUILD_BYPRODUCTS ${AZURE_STORAGE_BLOBS_STATIC_LIBRARY})
add_dependencies(Azure::azure-storage-blobs azure_storage_blobs_ep)

externalproject_add(azure_storage_common_ep
${EP_LOG_OPTIONS}
URL ${AZURE_STORAGE_COMMON_SOURCE_URL}
URL_HASH "SHA256=${ARROW_AZURE_STORAGE_COMMON_BUILD_SHA256_CHECKSUM}"
CMAKE_ARGS ${AZURESDK_CMAKE_ARGS}
BUILD_BYPRODUCTS ${AZURE_STORAGE_COMMON_STATIC_LIBRARY})
add_dependencies(Azure::azure-storage-common azure_storage_common_ep)

externalproject_add(azure_storage_files_datalake_ep
${EP_LOG_OPTIONS}
URL ${AZURE_STORAGE_FILES_DATALAKE_SOURCE_URL}
URL_HASH "SHA256=${ARROW_AZURE_STORAGE_FILES_DATALAKE_BUILD_SHA256_CHECKSUM}"
CMAKE_ARGS ${AZURESDK_CMAKE_ARGS}
BUILD_BYPRODUCTS ${AZURE_STORAGE_FILES_DATALAKE_STATIC_LIBRARY})
add_dependencies(Azure::azure-storage-files-datalake azure_storage_files_datalake_ep)

set_property(TARGET Azure::azure-core
APPEND
PROPERTY INTERFACE_LINK_LIBRARIES CURL::libcurl LibXml2::LibXml2)
Copy link
Copy Markdown
Member

@kou kou May 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to call find_curl() for CURL::libcurl and find_package(LibXml2 REQUIRED) for LibXml2::LibXml2.


set(AZURESDK_LINK_LIBRARIES ${AZURESDK_LIBRARIES})
endmacro()

if(ARROW_AZURE)
# TODO - use resolve_dependency
build_azuresdk()
foreach(AZURESDK_LIBRARY_CPP ${AZURESDK_LIBRARIES_CPP})
find_package(${AZURESDK_LIBRARY_CPP} CONFIG REQUIRED)
endforeach()
Copy link
Copy Markdown
Member

@kou kou May 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is needless.

include_directories(SYSTEM ${AZURESDK_INCLUDE_DIR})
Copy link
Copy Markdown
Member

@kou kou May 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is needless.

message(STATUS "Found Azure SDK headers: ${AZURESDK_INCLUDE_DIR}")
message(STATUS "Found Azure SDK libraries: ${AZURESDK_LINK_LIBRARIES}")
endif()

# ----------------------------------------------------------------------
# ucx - communication framework for modern, high-bandwidth and low-latency networks

Expand Down
28 changes: 28 additions & 0 deletions cpp/src/arrow/filesystem/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,34 @@ if(ARROW_GCS)
Boost::system)
endif()

if(ARROW_AZURE)
set(AZURE_SRCS)
list(APPEND
AZURE_SRCS
azurefs_mock.cc
azurefs.cc)

add_arrow_lib(azurefs
SOURCES
${AZURE_SRCS}
SHARED_LINK_LIBS
${ARROW_LINK_LIBS}
SHARED_PRIVATE_LINK_LIBS
${ARROW_SHARED_PRIVATE_LINK_LIBS}
STATIC_LINK_LIBS
${ARROW_STATIC_LINK_LIBS})

set_target_properties(azurefs_objlib PROPERTIES CXX_STANDARD 14 CXX_STANDARD_REQUIRED ON)
Copy link
Copy Markdown
Member

@kou kou May 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, sorry. I forgot that we embed filesystem modules to libarrow.so instead of creating separated libarrow_XXX.so. (We don't have separated CMake target for libarrow.so and filesystem modules.)

How about changing the default C++ version?

diff --git a/cpp/cmake_modules/SetupCxxFlags.cmake b/cpp/cmake_modules/SetupCxxFlags.cmake
index d3a2a1a2d2..cdaafe379b 100644
--- a/cpp/cmake_modules/SetupCxxFlags.cmake
+++ b/cpp/cmake_modules/SetupCxxFlags.cmake
@@ -118,12 +118,16 @@ if(NOT DEFINED CMAKE_C_STANDARD)
   set(CMAKE_C_STANDARD 11)
 endif()
 
-# This ensures that things like c++11 get passed correctly
+# This ensures that things like c++11/c++14 get passed correctly
 if(NOT DEFINED CMAKE_CXX_STANDARD)
-  set(CMAKE_CXX_STANDARD 11)
+  if(ARROW_AZURE)
+    set(CMAKE_CXX_STANDARD 14)
+  else()
+    set(CMAKE_CXX_STANDARD 11)
+  endif()
 endif()
 
-# We require a C++11 compliant compiler
+# We require a C++11/14 compliant compiler
 set(CMAKE_CXX_STANDARD_REQUIRED ON)
 
 # ARROW-6848: Do not use GNU (or other CXX) extensions
diff --git a/cpp/src/arrow/CMakeLists.txt b/cpp/src/arrow/CMakeLists.txt
index ec6cada1cd..1ded8e59d4 100644
--- a/cpp/src/arrow/CMakeLists.txt
+++ b/cpp/src/arrow/CMakeLists.txt
@@ -469,6 +469,12 @@ if(ARROW_FILESYSTEM)
        filesystem/path_util.cc
        filesystem/util_internal.cc)
 
+  if(ARROW_AZURE)
+    list(APPEND ARROW_SRCS filesystem/azurefs.cc filesystem/azurefs_mock.cc)
+    set_source_files_properties(filesystem/azurefs.cc filesystem/azurefs_mock.cc
+                                PROPERTIES SKIP_PRECOMPILE_HEADERS ON
+                                           SKIP_UNITY_BUILD_INCLUSION ON)
+  endif()
   if(ARROW_GCS)
     list(APPEND ARROW_SRCS filesystem/gcsfs.cc filesystem/gcsfs_internal.cc)
     set_source_files_properties(filesystem/gcsfs.cc filesystem/gcsfs_internal.cc
diff --git a/cpp/src/arrow/filesystem/CMakeLists.txt b/cpp/src/arrow/filesystem/CMakeLists.txt
index 819eca08cf..bbca231baf 100644
--- a/cpp/src/arrow/filesystem/CMakeLists.txt
+++ b/cpp/src/arrow/filesystem/CMakeLists.txt
@@ -28,8 +28,8 @@ add_arrow_test(filesystem-test
                EXTRA_LABELS
                filesystem)
 
-if(ARROW_GCS)
-  add_arrow_test(gcsfs_test
+if(ARROW_AZURE)
+  add_arrow_test(azurefs_test
                  EXTRA_LABELS
                  filesystem
                  EXTRA_LINK_LIBS
@@ -37,32 +37,13 @@ if(ARROW_GCS)
                  Boost::system)
 endif()
 
-if(ARROW_AZURE)
-  set(AZURE_SRCS)
-  list(APPEND
-        AZURE_SRCS
-        azurefs_mock.cc
-        azurefs.cc)
-
-  add_arrow_lib(azurefs
-                SOURCES
-                ${AZURE_SRCS}
-                SHARED_LINK_LIBS
-                ${ARROW_LINK_LIBS}
-                SHARED_PRIVATE_LINK_LIBS
-                ${ARROW_SHARED_PRIVATE_LINK_LIBS}
-                STATIC_LINK_LIBS
-                ${ARROW_STATIC_LINK_LIBS})
-
-  set_target_properties(azurefs_objlib PROPERTIES CXX_STANDARD 14 CXX_STANDARD_REQUIRED ON)
-
-  set(TEST_LIBS_AZURE ${ARROW_TEST_LINK_LIBS})
-  list(APPEND TEST_LIBS_AZURE azurefs_shared)
-  add_arrow_test(azurefs_test EXTRA_LABELS filesystem
-                  STATIC_LINK_LIBS
-                  ${TEST_LIBS_AZURE}
-  )
-  set_target_properties(arrow-azurefs-test PROPERTIES CXX_STANDARD 14 CXX_STANDARD_REQUIRED ON)
+if(ARROW_GCS)
+  add_arrow_test(gcsfs_test
+                 EXTRA_LABELS
+                 filesystem
+                 EXTRA_LINK_LIBS
+                 Boost::filesystem
+                 Boost::system)
 endif()
 
 if(ARROW_S3)

Copy link
Copy Markdown
Author

@shefali163 shefali163 May 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we change the default C++ version, then it will pick C++14 for the entire compilation, and looks like from this conversation - https://issues.apache.org/jira/browse/ARROW-2034?focusedCommentId=17463318&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17463318, it is not desired, right?

Copy link
Copy Markdown
Member

@kou kou May 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that @pitrou said that we can use C++14 features in cpp/src/arrow/filesystem/azurefs*.cc but we must not use C++14 features in other *.cc.

Copy link
Copy Markdown
Member

@pitrou pitrou May 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. Also, .h files should not use any C++14 features either.


set(TEST_LIBS_AZURE ${ARROW_TEST_LINK_LIBS})
list(APPEND TEST_LIBS_AZURE azurefs_shared)
add_arrow_test(azurefs_test EXTRA_LABELS filesystem
STATIC_LINK_LIBS
${TEST_LIBS_AZURE}
)
set_target_properties(arrow-azurefs-test PROPERTIES CXX_STANDARD 14 CXX_STANDARD_REQUIRED ON)
endif()

if(ARROW_S3)
add_arrow_test(s3fs_test
SOURCES
Expand Down
Loading