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

[DocDB] Correctly manage shared library dependencies for Postgres extensions #17203

Open
1 task done
mbautin opened this issue May 5, 2023 · 1 comment
Open
1 task done
Assignees
Labels
area/docdb YugabyteDB core features kind/enhancement This is an enhancement of an existing feature priority/medium Medium priority issue

Comments

@mbautin
Copy link
Collaborator

mbautin commented May 5, 2023

Jira Link: DB-6465

Description

E.g. we should not copy pg_stat_statements.so to the bin directory.

Warning: Please confirm that this issue does not contain any sensitive information

  • I confirm this issue does not contain any sensitive information.
@mbautin mbautin added area/docdb YugabyteDB core features status/awaiting-triage Issue awaiting triage labels May 5, 2023
@mbautin mbautin self-assigned this May 5, 2023
@yugabyte-ci yugabyte-ci added kind/bug This issue is a bug priority/medium Medium priority issue labels May 5, 2023
@svarnau
Copy link
Member

svarnau commented May 5, 2023

What is special about this library and why is it an exception?

@yugabyte-ci yugabyte-ci added kind/enhancement This is an enhancement of an existing feature and removed status/awaiting-triage Issue awaiting triage kind/bug This issue is a bug labels May 9, 2023
mbautin added a commit that referenced this issue May 11, 2023
Summary:
After 01328da it became necessary to look at shared library dependencies of Postgres libraries in library_packager, because in some cases required libraries could only be discovered that way. For example, our version of the Postgres extension pg_stat_statements.so depends on libyb_pggate_webserver.so, but no other executables or shared libraries directly reachable from the "seed executables" specified in the "bin" section of yb_release_manifest.json depend on that library.

With this diff, for Linuxbrew builds, we are setting rpaths for files in the postgres/lib directory in post_install.sh, and removing rpaths from the corresponding files at the packaging step, so Postgres binaries in the .tar.gz do not have rpaths. We also look at all files in the postgres/lib directory when discovering the shared libraries to install, so pg_stat_statements.so's dependencies are packaged.

mac_library_packager.py requires a different fix from the above solution, so keeping the old approach of adding Postgres libraries to "seed executable patterns" for now.

Also fixing another issue that is breaking itest: sys.path should be set differently in gen_version_info.py after it was moved from build-support to python/yugabyte in 9d99f77.

Test Plan:
Jenkins: compile only

Manual testing:

./yb_release --force --keep_tmp_dir --skip_build --skip_yugabyted_ui

And then examine the temporary directories generated by the build, including attempting to run post_install.sh and yb-ctl from there.

Also check that after running bin/post_install.sh, we don't get any shared library dependencies that resolve to files in the build directory, meaning that original rpaths are properly removed in the Linuxbrew build. The easiest way to check for this is to grep for "clang15" which is included in the build directory's name.

find . -type f -exec ldd {} \; |& grep clang15

Reviewers: sergei, steve.varnau, jharveysmith

Reviewed By: jharveysmith

Subscribers: jenkins-bot, yugaware, ybase

Differential Revision: https://phabricator.dev.yugabyte.com/D25109
acuskev pushed a commit that referenced this issue Jun 5, 2023
…t_statements.so

Summary:
Original Commit: 532319e / D25109

# Commit message from D25109:

After 01328da it became necessary to look at shared library dependencies of Postgres libraries in library_packager, because in some cases required libraries could only be discovered that way. For example, our version of the Postgres extension pg_stat_statements.so depends on libyb_pggate_webserver.so, but no other executables or shared libraries directly reachable from the "seed executables" specified in the "bin" section of yb_release_manifest.json depend on that library.

With this diff, for Linuxbrew builds, we are setting rpaths for files in the postgres/lib directory in post_install.sh, and removing rpaths from the corresponding files at the packaging step, so Postgres binaries in the .tar.gz do not have rpaths. We also look at all files in the postgres/lib directory when discovering the shared libraries to install, so pg_stat_statements.so's dependencies are packaged.

mac_library_packager.py requires a different fix from the above solution, so keeping the old approach of adding Postgres libraries to "seed executable patterns" for now.

Also fixing another issue that is breaking itest: sys.path should be set differently in gen_version_info.py after it was moved from build-support to python/yugabyte in 9d99f77.

## Additional changes:
Brought in changes to mac_library_packager.py and the yb-ctl script from 01328da / D25113

Specifically, removed all references to yb_lib_file_for_postgres in mac_library_packager.py and removed all references to run_cluster_wide_ysql_initdb from yb-ctl. This is because the applied changes from D25109 overwrite the need to explicitly add libraries like libyb_pggate.dylib that are referenced in pg_stat_statements.

Test Plan:
Jenkins: compile only

Manual testing:

./yb_release --force --keep_tmp_dir --skip_build --skip_yugabyted_ui

And then examine the temporary directories generated by the build, including attempting to run post_install.sh and yb-ctl from there.

Also check that after running bin/post_install.sh, we don't get any shared library dependencies that resolve to files in the build directory, meaning that original rpaths are properly removed in the Linuxbrew build. The easiest way to check for this is to grep for "clang15" which is included in the build directory's name.

find . -type f -exec ldd {} \; |& grep clang15

Reviewers: sergei, steve.varnau, jharveysmith, mbautin

Reviewed By: mbautin

Subscribers: ybase, yugaware, jenkins-bot

Differential Revision: https://phorge.dev.yugabyte.com/D25956
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/docdb YugabyteDB core features kind/enhancement This is an enhancement of an existing feature priority/medium Medium priority issue
Projects
None yet
Development

No branches or pull requests

3 participants