Skip to content

Commit

Permalink
Fix "unknown pool name 'link_job_pool'" CMake error
Browse files Browse the repository at this point in the history
Velox creates link_job_pool only when MAX_LINK_JOBS is defined, but by
default, this variable is not defined, and thus the CMake job pool was
not created. However in presto_cpp, this job pool was used without
checking its existence, and caused the "unknown pool name 'link_job_pool'"
error while loading the CMake project. This commit fixes this problem
by adding a check whether the job pool was created before using it.
  • Loading branch information
yingsu00 committed Oct 11, 2023
1 parent 6c9b457 commit a2f82f2
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 7 deletions.
11 changes: 8 additions & 3 deletions presto-native-execution/presto_cpp/main/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,10 @@ target_link_libraries(
# dependencies first followed by FBThrift.
target_link_libraries(presto_server_lib presto_thrift-cpp2 presto_thrift_extra
${THRIFT_LIBRARY})

set_property(TARGET presto_server_lib PROPERTY JOB_POOL_LINK link_job_pool)
message("Checking CMAKE_JOB_POOL_LINK: ${CMAKE_JOB_POOL_LINK}")
if("${CMAKE_JOB_POOL_LINK}")
set_property(TARGET presto_server_lib PROPERTY JOB_POOL_LINK link_job_pool)
endif()

add_executable(presto_server PrestoMain.cpp)

Expand All @@ -97,7 +99,10 @@ if(PRESTO_ENABLE_REMOTE_FUNCTIONS)
target_link_libraries(presto_server_lib presto_server_remote_function)
endif()

set_property(TARGET presto_server PROPERTY JOB_POOL_LINK link_job_pool)
message("Checking CMAKE_JOB_POOL_LINK: ${CMAKE_JOB_POOL_LINK}")
if("${CMAKE_JOB_POOL_LINK}")
set_property(TARGET presto_server PROPERTY JOB_POOL_LINK link_job_pool)
endif()

if(PRESTO_ENABLE_TESTING)
add_subdirectory(tests)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ add_library(presto_exception Exception.cpp)
add_library(presto_common Counters.cpp Utils.cpp ConfigReader.cpp Configs.cpp)

target_link_libraries(presto_exception velox_exception)
set_property(TARGET presto_exception PROPERTY JOB_POOL_LINK link_job_pool)
if("${CMAKE_JOB_POOL_LINK}")
set_property(TARGET presto_exception PROPERTY JOB_POOL_LINK link_job_pool)
endif()

target_link_libraries(presto_common velox_config velox_core velox_exception)

Expand Down
4 changes: 3 additions & 1 deletion presto-native-execution/presto_cpp/main/http/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@ target_link_libraries(
${FMT}
${RE2})

set_property(TARGET presto_http PROPERTY JOB_POOL_LINK link_job_pool)
if("${CMAKE_JOB_POOL_LINK}")
set_property(TARGET presto_http PROPERTY JOB_POOL_LINK link_job_pool)
endif()

if(PRESTO_ENABLE_TESTING)
add_subdirectory(tests)
Expand Down
4 changes: 3 additions & 1 deletion presto-native-execution/presto_cpp/main/tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,9 @@ target_link_libraries(
gtest
gtest_main)

set_property(TARGET presto_server_test PROPERTY JOB_POOL_LINK link_job_pool)
if("${CMAKE_JOB_POOL_LINK}")
set_property(TARGET presto_server_test PROPERTY JOB_POOL_LINK link_job_pool)
endif()

if(PRESTO_ENABLE_REMOTE_FUNCTIONS)
add_executable(presto_server_remote_function_test
Expand Down
4 changes: 3 additions & 1 deletion presto-native-execution/presto_cpp/main/types/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ add_dependencies(presto_types presto_operators presto_type_converter velox_type
target_link_libraries(presto_types presto_type_converter velox_type_fbhive
velox_hive_partition_function velox_tpch_gen)

set_property(TARGET presto_types PROPERTY JOB_POOL_LINK link_job_pool)
if("${CMAKE_JOB_POOL_LINK}")
set_property(TARGET presto_types PROPERTY JOB_POOL_LINK link_job_pool)
endif()

if(PRESTO_ENABLE_TESTING)
add_subdirectory(tests)
Expand Down

0 comments on commit a2f82f2

Please sign in to comment.