Re: [PR] GH-41004: [C++][FS][Azure] Don't run TestGetFileInfoGenerator() with Valgrind [arrow]

2024-04-20 Thread via GitHub
kou commented on PR #41163: URL: https://github.com/apache/arrow/pull/41163#issuecomment-2067874304 It seems that `--with-tls` is related: https://github.com/apache/arrow/pull/41279#issuecomment-2067874205 -- This is an automated message from the Apache Git Service. To respond to the

Re: [PR] GH-41004: [C++][FS][Azure] Don't run TestGetFileInfoGenerator() with Valgrind [arrow]

2024-04-15 Thread via GitHub
pitrou commented on PR #41163: URL: https://github.com/apache/arrow/pull/41163#issuecomment-2056957706 It's more a case of DLL finalization coming before whatever unit of work is still being executed in some worker thread, AFAIU. We had a similar problem with S3 (but resulting in worse

Re: [PR] GH-41004: [C++][FS][Azure] Don't run TestGetFileInfoGenerator() with Valgrind [arrow]

2024-04-15 Thread via GitHub
felipecrv commented on PR #41163: URL: https://github.com/apache/arrow/pull/41163#issuecomment-2056949564 Looks like yet another case of a library polluting the thread-local storage area :) -- This is an automated message from the Apache Git Service. To respond to the message, please log

Re: [PR] GH-41004: [C++][FS][Azure] Don't run TestGetFileInfoGenerator() with Valgrind [arrow]

2024-04-15 Thread via GitHub
pitrou commented on PR #41163: URL: https://github.com/apache/arrow/pull/41163#issuecomment-205584 > I think that `--with-tls` isn't the cause of this. Well, we can wait for an update conda-forge package to make sure that's the case. -- This is an automated message from the

Re: [PR] GH-41004: [C++][FS][Azure] Don't run TestGetFileInfoGenerator() with Valgrind [arrow]

2024-04-13 Thread via GitHub
kou commented on PR #41163: URL: https://github.com/apache/arrow/pull/41163#issuecomment-2053787467 I've also investigated this more. I think that `--with-tls` isn't the cause of this. I could reproduce this by the following program that only uses `libxml2` and `std::thread`:

Re: [PR] GH-41004: [C++][FS][Azure] Don't run TestGetFileInfoGenerator() with Valgrind [arrow]

2024-04-12 Thread via GitHub
kou commented on code in PR #41163: URL: https://github.com/apache/arrow/pull/41163#discussion_r1563245965 ## cpp/src/arrow/util/config.h.cmake: ## @@ -57,6 +57,7 @@ #cmakedefine ARROW_GCS #cmakedefine ARROW_HDFS #cmakedefine ARROW_S3 +#cmakedefine ARROW_TEST_MEMCHECK

Re: [PR] GH-41004: [C++][FS][Azure] Don't run TestGetFileInfoGenerator() with Valgrind [arrow]

2024-04-12 Thread via GitHub
pitrou commented on PR #41163: URL: https://github.com/apache/arrow/pull/41163#issuecomment-2051897809 I've reported https://github.com/conda-forge/libxml2-feedstock/issues/117 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub

Re: [PR] GH-41004: [C++][FS][Azure] Don't run TestGetFileInfoGenerator() with Valgrind [arrow]

2024-04-12 Thread via GitHub
pitrou commented on PR #41163: URL: https://github.com/apache/arrow/pull/41163#issuecomment-2051880913 Ok, that's because `--with-tls` doesn't get passed here: https://github.com/conda-forge/libxml2-feedstock/blob/main/recipe/build.sh -- This is an automated message from the Apache

Re: [PR] GH-41004: [C++][FS][Azure] Don't run TestGetFileInfoGenerator() with Valgrind [arrow]

2024-04-12 Thread via GitHub
pitrou commented on PR #41163: URL: https://github.com/apache/arrow/pull/41163#issuecomment-2051877190 Judging by the disassembly of `xmlGetThreadLocalStorage` on conda-forge's libxml2, it seems that `USE_TLS` doesn't get enabled in that package. -- This is an automated message from the

Re: [PR] GH-41004: [C++][FS][Azure] Don't run TestGetFileInfoGenerator() with Valgrind [arrow]

2024-04-12 Thread via GitHub
felipecrv commented on PR #41163: URL: https://github.com/apache/arrow/pull/41163#issuecomment-2051875729 And the Azure SDK depends on it: ``` _deps/azure_sdk-build/sdk/storage/azure-storage-common/azure-storage-common-cppConfig.cmake 35: find_dependency(LibXml2) ``` --

Re: [PR] GH-41004: [C++][FS][Azure] Don't run TestGetFileInfoGenerator() with Valgrind [arrow]

2024-04-12 Thread via GitHub
pitrou commented on PR #41163: URL: https://github.com/apache/arrow/pull/41163#issuecomment-2051867304 > Where does `libxml2` come in our case? Is it vendored by the Azure SDK? Something else? Answering myself: it's version 2.12.6 from conda-forge. -- This is an automated message

Re: [PR] GH-41004: [C++][FS][Azure] Don't run TestGetFileInfoGenerator() with Valgrind [arrow]

2024-04-12 Thread via GitHub
pitrou commented on PR #41163: URL: https://github.com/apache/arrow/pull/41163#issuecomment-2051834340 Note that `xmlNewGlobalState` seems to be some kind of legacy function ("backwards compatibility of libxml2 to pre-thread-safe behaviour"):

Re: [PR] GH-41004: [C++][FS][Azure] Don't run TestGetFileInfoGenerator() with Valgrind [arrow]

2024-04-12 Thread via GitHub
pitrou commented on PR #41163: URL: https://github.com/apache/arrow/pull/41163#issuecomment-2051819043 > But I suspect it's something in the Azure SDK (exception-safety failure?) because `GetFileInfoWithSelectorFromContainer` doesn't do complicated memory management. That may

Re: [PR] GH-41004: [C++][FS][Azure] Don't run TestGetFileInfoGenerator() with Valgrind [arrow]

2024-04-12 Thread via GitHub
felipecrv commented on PR #41163: URL: https://github.com/apache/arrow/pull/41163#issuecomment-2051811767 > Ok... This seems to assume that Valgrind is buggy and doesn't handle a calloc() call correctly, while our Azure FS implementation (and/or the Azure SDK) is spotless and cannot

Re: [PR] GH-41004: [C++][FS][Azure] Don't run TestGetFileInfoGenerator() with Valgrind [arrow]

2024-04-12 Thread via GitHub
pitrou commented on PR #41163: URL: https://github.com/apache/arrow/pull/41163#issuecomment-2051322908 Ok... This seems to assume that Valgrind is buggy and doesn't handle a calloc() call correctly, while our Azure FS implementation (and/or the Azure SDK) is spotless and cannot contain any

Re: [PR] GH-41004: [C++][FS][Azure] Don't run TestGetFileInfoGenerator() with Valgrind [arrow]

2024-04-12 Thread via GitHub
kou commented on code in PR #41163: URL: https://github.com/apache/arrow/pull/41163#discussion_r1562240202 ## cpp/src/arrow/util/config.h.cmake: ## @@ -57,6 +57,7 @@ #cmakedefine ARROW_GCS #cmakedefine ARROW_HDFS #cmakedefine ARROW_S3 +#cmakedefine ARROW_TEST_MEMCHECK

Re: [PR] GH-41004: [C++][FS][Azure] Don't run TestGetFileInfoGenerator() with Valgrind [arrow]

2024-04-12 Thread via GitHub
kou commented on PR #41163: URL: https://github.com/apache/arrow/pull/41163#issuecomment-2051302206 My understanding is https://github.com/apache/arrow/pull/40567/files#diff-7d2cbf9c6abf1ee980b8cf5a79e222543c17b3d93b12c4c0c9fa123b1bb200b6R398-R411 . -- This is an automated message from

Re: [PR] GH-41004: [C++][FS][Azure] Don't run TestGetFileInfoGenerator() with Valgrind [arrow]

2024-04-12 Thread via GitHub
pitrou commented on code in PR #41163: URL: https://github.com/apache/arrow/pull/41163#discussion_r1562232462 ## cpp/src/arrow/util/config.h.cmake: ## @@ -57,6 +57,7 @@ #cmakedefine ARROW_GCS #cmakedefine ARROW_HDFS #cmakedefine ARROW_S3 +#cmakedefine ARROW_TEST_MEMCHECK

Re: [PR] GH-41004: [C++][FS][Azure] Don't run TestGetFileInfoGenerator() with Valgrind [arrow]

2024-04-12 Thread via GitHub
kou commented on code in PR #41163: URL: https://github.com/apache/arrow/pull/41163#discussion_r1562230914 ## cpp/src/arrow/util/config.h.cmake: ## @@ -57,6 +57,7 @@ #cmakedefine ARROW_GCS #cmakedefine ARROW_HDFS #cmakedefine ARROW_S3 +#cmakedefine ARROW_TEST_MEMCHECK

Re: [PR] GH-41004: [C++][FS][Azure] Don't run TestGetFileInfoGenerator() with Valgrind [arrow]

2024-04-12 Thread via GitHub
pitrou commented on PR #41163: URL: https://github.com/apache/arrow/pull/41163#issuecomment-2051262752 @kou Did we try to understand _why_ a memory leak was reported? We shouldn't just silence the errors that annoy us. cc @felipecrv -- This is an automated message from the Apache

Re: [PR] GH-41004: [C++][FS][Azure] Don't run TestGetFileInfoGenerator() with Valgrind [arrow]

2024-04-12 Thread via GitHub
pitrou commented on code in PR #41163: URL: https://github.com/apache/arrow/pull/41163#discussion_r1562204543 ## cpp/src/arrow/util/config.h.cmake: ## @@ -57,6 +57,7 @@ #cmakedefine ARROW_GCS #cmakedefine ARROW_HDFS #cmakedefine ARROW_S3 +#cmakedefine ARROW_TEST_MEMCHECK

Re: [PR] GH-41004: [C++][FS][Azure] Don't run TestGetFileInfoGenerator() with Valgrind [arrow]

2024-04-12 Thread via GitHub
conbench-apache-arrow[bot] commented on PR #41163: URL: https://github.com/apache/arrow/pull/41163#issuecomment-2051216955 After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit 6e8ac430816b84d295ddf89709c4b42e558b2f7e. There were

Re: [PR] GH-41004: [C++][FS][Azure] Don't run TestGetFileInfoGenerator() with Valgrind [arrow]

2024-04-11 Thread via GitHub
kou merged PR #41163: URL: https://github.com/apache/arrow/pull/41163 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail:

Re: [PR] GH-41004: [C++][FS][Azure] Don't run TestGetFileInfoGenerator() with Valgrind [arrow]

2024-04-11 Thread via GitHub
kou commented on PR #41163: URL: https://github.com/apache/arrow/pull/41163#issuecomment-2050860758 +1 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe,

Re: [PR] GH-41004: [C++][FS][Azure] Don't run TestGetFileInfoGenerator() with Valgrind [arrow]

2024-04-11 Thread via GitHub
github-actions[bot] commented on PR #41163: URL: https://github.com/apache/arrow/pull/41163#issuecomment-2050826523 Revision: 5f4255fc3f37b6352527847befcfd848fe8a7d05 Submitted crossbow builds: [ursacomputing/crossbow @

Re: [PR] GH-41004: [C++][FS][Azure] Don't run TestGetFileInfoGenerator() with Valgrind [arrow]

2024-04-11 Thread via GitHub
kou commented on PR #41163: URL: https://github.com/apache/arrow/pull/41163#issuecomment-2050824817 @github-actions crossbow submit test-conda-cpp-valgrind -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above

Re: [PR] GH-41004: [C++][FS][Azure] Don't run TestGetFileInfoGenerator() with Valgrind [arrow]

2024-04-11 Thread via GitHub
github-actions[bot] commented on PR #41163: URL: https://github.com/apache/arrow/pull/41163#issuecomment-2050824889 :warning: GitHub issue #41004 **has been automatically assigned in GitHub** to PR creator. -- This is an automated message from the Apache Git Service. To respond to the