Skip to content
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.

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

[VL][Doc][Build]Ubuntu 24.04 support - Test + Fix + Doc #7977

Open
yabinma opened this issue Nov 18, 2024 · 3 comments
Open

[VL][Doc][Build]Ubuntu 24.04 support - Test + Fix + Doc #7977

yabinma opened this issue Nov 18, 2024 · 3 comments
Assignees
Labels
enhancement New feature or request

Comments

@yabinma
Copy link
Contributor

yabinma commented Nov 18, 2024

Background:

In the Getting-Started document, it's mentioned that Ubuntu 20.04/22.04 are supported. While Ubuntu 24.04 LTS was released on April 2024 and it's a long term supported version.

This ticket is for tracking the test on Ubuntu 24.04 and it will list the issues met and it will enhance the document once the test done.

Build Test:

prerequisite:
cmake, openjdk-8, ninja, mvn

sudo apt update & sudo apt install -y build-essential openjdk-8-jdk cmake ninja-build

gcc v13.2 will be installed and this is the major difference with Ubuntu 22.04(gcc v11.4)

maven download link : https://maven.apache.org/download.cgi
for example maven v3.8.8 : https://dlcdn.apache.org/maven/maven-3/3.8.8/binaries/apache-maven-3.8.8-bin.tar.gz

After download, tar xvzf apache-maven-3.8.8-bin.tar.gz unzip to a folder for example, /opt/apache-maven-3.8.8

After install completion, edit /etc/profile to add JAVA_HOME and M2_HOME.
vi /etc/profile, to add,

export M2_HOME=/opt/apache-maven-3.8.8
export JAVA_HOME=/usr/lib/jvm/java-8-openjdk-amd64
export PATH=$M2_HOME/bin:$JAVA_HOME/bin:$PATH

source /etc/profile to load the profile for current user.

Build:
As the getting-started doc mentioned, ./dev/buildbundle-veloxbe.sh is used to build in one script.
Two ENV variables are recommended, for example,

TREAT_WARNINGS_AS_ERRORS=0 NUM_THREADS=4 ./dev/buildbundle-veloxbe.sh

By default, all warnings are treated as errors. TREAT_WARNINGS_AS_ERRORS=0 is to disable it and to avoid build error because of warnings.
NUM_THREADS=X is to specify the CPU core in compile. This is to avoid the OOM issue in compile.

build issue 1:

/opt/gitspace/incubator-gluten/ep/build-velox/build/velox_ep/velox/functions/prestosql/InPredicate.cpp: In lambda function:
/opt/gitspace/incubator-gluten/ep/build-velox/build/velox_ep/velox/functions/prestosql/InPredicate.cpp:94:50: error: call of overloaded ‘contains(<brace-enclosed initializer list>)’ is ambiguous
   94 |         const bool found = uniqueValues_.contains({arg.get(), row});

It does not happen with gcc v11.4.
With gcc 13.2, the implicit type caused the overload error.
A code fix is opened in PR: facebookincubator/velox#11573 ---Merged
The fix is to specify the type explicitly.

build issue 2:

In file included from /opt/gitspace/incubator-gluten/ep/build-velox/build/velox_ep/velox/exec/Trace.cpp:17:
/opt/gitspace/incubator-gluten/ep/build-velox/build/velox_ep/./velox/exec/Trace.h:48:3: error: ‘uint64_t’ does not name a type
   48 |   uint64_t inputRows{0};
      |   ^~~~~~~~
/opt/gitspace/incubator-gluten/ep/build-velox/build/velox_ep/./velox/exec/Trace.h:20:1: note: ‘uint64_t’ is defined in header ‘<cstdint>’; did you forget to ‘#include <cstdint>’?
   19 | #include <string>
  +++ |+#include <cstdint>

It does not happen with gcc v11.4.
Code fix in PR: facebookincubator/velox#11594 ---Merged

build issue 3:

In file included from /opt/gitspace/incubator-gluten/dev/../ep/build-velox/build/velox_ep/velox/dwio/common/BufferedInput.h:22,
                 from /opt/gitspace/incubator-gluten/dev/../ep/build-velox/build/velox_ep/velox/dwio/common/ReaderFactory.h:21,
                 from /opt/gitspace/incubator-gluten/cpp/velox/operators/writer/VeloxParquetDataSource.h:51,
                 from /opt/gitspace/incubator-gluten/cpp/velox/compute/VeloxRuntime.h:25,
                 from /opt/gitspace/incubator-gluten/cpp/velox/compute/VeloxBackend.cc:33:
/opt/gitspace/incubator-gluten/dev/../ep/build-velox/build/velox_ep/velox/dwio/common/StreamIdentifier.h:40:16: error: ‘virtual bool facebook::velox::dwio::common::StreamIdentifier::operator==(const facebook::velox::dwio::common::StreamIdentifier&) const’ was hidden [-Werror=overloaded-virtual=]
   40 |   virtual bool operator==(const StreamIdentifier& other) const {
      |                ^~~~~~~~
In file included from /opt/gitspace/incubator-gluten/dev/../ep/build-velox/build/velox_ep/velox/dwio/dwrf/common/ByteRLE.h:27,
                 from /opt/gitspace/incubator-gluten/dev/../ep/build-velox/build/velox_ep/velox/dwio/dwrf/reader/ColumnReader.h:24,
                 from /opt/gitspace/incubator-gluten/dev/../ep/build-velox/build/velox_ep/velox/dwio/dwrf/reader/SelectiveDwrfReader.h:20,
                 from /opt/gitspace/incubator-gluten/dev/../ep/build-velox/build/velox_ep/velox/dwio/dwrf/reader/DwrfReader.h:23,
                 from /opt/gitspace/incubator-gluten/dev/../ep/build-velox/build/velox_ep/velox/dwio/orc/reader/OrcReader.h:20,
                 from /opt/gitspace/incubator-gluten/cpp/velox/compute/VeloxBackend.cc:47:
/opt/gitspace/incubator-gluten/dev/../ep/build-velox/build/velox_ep/velox/dwio/dwrf/common/Common.h:196:8: note:   by ‘bool facebook::velox::dwrf::DwrfStreamIdentifier::operator==(const facebook::velox::dwrf::DwrfStreamIdentifier&) const’
  196 |   bool operator==(const DwrfStreamIdentifier& other) const {
      |        ^~~~~~~~

It does not happen with gcc v11.4.
Code fix in PR: facebookincubator/velox#11641

build issue 4:

error: ‘uint64_t’ does not name a type

It does not happen with gcc v11.4.
Code fix in PR: #8030 ---Merged

build issue 5:

error: ‘uint64_t’ does not name a type in libhdfs3

https://github.com/oap-project/libhdfs3
It does not happen with gcc v11.4.
I am not sure if fixes for libhdfs3, there is a discussion in facebookincubator/velox#9969
It's planning to replace libhdfs3 with JVM based libhdfs.
Bypass libhdfs3 in the test. There is another PR for removing libhdfs3, : #8013

Doc/script draft update done in fork, TODO: E2E test after the PRs merged(listed above).

@yabinma yabinma added the enhancement New feature or request label Nov 18, 2024
@yabinma
Copy link
Contributor Author

yabinma commented Nov 18, 2024

Hello Team, anyone please help assign the ticket to me? I will keep adding the test result and any fix required. Thanks.

@PHILO-HE
Copy link
Contributor

@yabinma, just assigned this issue to you.

@yabinma yabinma changed the title [Doc][Build]Ubuntu 24.04 support - Test + Doc [Doc][Build]Ubuntu 24.04 support - Test + Fix + Doc Nov 19, 2024
@FelixYBW FelixYBW changed the title [Doc][Build]Ubuntu 24.04 support - Test + Fix + Doc [VL][Doc][Build]Ubuntu 24.04 support - Test + Fix + Doc Nov 19, 2024
@FelixYBW
Copy link
Contributor

Thank you for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants