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

[YSQL] Fix ASAN issues with backfill libpq connection #6481

Closed
jaki opened this issue Nov 24, 2020 · 0 comments
Closed

[YSQL] Fix ASAN issues with backfill libpq connection #6481

jaki opened this issue Nov 24, 2020 · 0 comments
Assignees
Labels
area/ysql Yugabyte SQL (YSQL)

Comments

@jaki
Copy link
Contributor

jaki commented Nov 24, 2020

The index backfill libpq connection created in tablet.cc uses the raw C funcs, so it is easy to miss some spots where objects should be freed, and ASAN failures abound. Create and link a library containing the pgwrapper C++ functions with destructors so that these issues can be more easily avoided.

@jaki jaki added the area/ysql Yugabyte SQL (YSQL) label Nov 24, 2020
@jaki jaki self-assigned this Nov 24, 2020
@jaki jaki added this to To do/Current-Sprint in Index backfill via automation Nov 24, 2020
@jaki jaki moved this from To do/Current-Sprint to In progress in Index backfill Nov 24, 2020
@jaki jaki closed this as completed in 5243a4a Nov 26, 2020
Index backfill automation moved this from In progress to Done Nov 26, 2020
jaki added a commit that referenced this issue Dec 10, 2020
Summary:

We have `libpq_utils` that use the PQ library to create connections and
manage them using C++ objects.  They are handy for making sure we clear
results and connections after use.  There are some issues around this
area:

- YSQL index backfill doesn't use them and just uses the raw PQ
  library.  There have been some mistakes causing ASAN issues
- User name is hard-coded to `postgres` for `PGConn::Connect`
- Database name and user name aren't properly escaped, so weird names
  may cause issues

Fix all of them.  Notably, split off part of the `pg_wrapper_test_base`
library to a new library `pq_utils` so that `tablet` can use `pq_utils`
as a dependency without getting a dependency cycle.  Then, YSQL index
backfill uses the C++ wrapped PQ objects.

Original Commit: 5243a4a

Original Differential Revision: https://phabricator.dev.yugabyte.com/D9916

Test Plan:

Jenkins: rebase: 2.4

`./yb_build.sh --cxx-test pgwrapper_lib_pq-test --gtest_filter
'PgLibPqTest.*Names'`

Reviewers: mbautin

Reviewed By: mbautin

Subscribers: yql

Differential Revision: https://phabricator.dev.yugabyte.com/D10041
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ysql Yugabyte SQL (YSQL)
Projects
Development

No branches or pull requests

1 participant