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

WAL/blockmaps: continue blockmaps initialization after failure #211

Closed
wants to merge 18 commits into from

Conversation

medvied
Copy link
Contributor

@medvied medvied commented Oct 21, 2023

ContinueBlockMapGroupInitialization() description has all the details about how it's done and how different kinds of failures are handled.

ContinueBlockMapGroupInitialization() description has all the details
about how it's done and how different kinds of failures are handled.
medvied added a commit to medvied/lantern that referenced this pull request Oct 25, 2023
An issue with PG_VERSION_NUM definition for PostgreSQL < 15 has been
revealed by lanterndata#211: PG_VERSION_NUM is defined for PostgreSQL 16.

Inside the build environment for PostgreSQL 15 it looks like the
following:

```
/usr/include/postgresql/pg_config.h:#define PG_VERSION_NUM 160000
/usr/include/postgresql/15/server/pg_config.h:#define PG_VERSION_NUM 150004
/usr/include/postgresql/15/server/fmgr.h:       PG_VERSION_NUM / 100, \
```

And the build command for, for example, `src/hnsw/external_index.c`
looks like this:

```
/usr/bin/cc -DLANTERN_USE_USEARCH -Dlantern_EXPORTS
-I/tmp/lantern/./third_party/usearch/c -isystem /usr/include/postgresql
-isystem /usr/include/postgresql/15/server -isystem /tmp/lantern/src
-isystem /tmp/lantern -Wall -Wextra -Wno-conversion -Wno-unknown-pragmas
-fPIC -MD -MT CMakeFiles/lantern.dir/src/hnsw/external_index.c.o -MF
CMakeFiles/lantern.dir/src/hnsw/external_index.c.o.d -o
CMakeFiles/lantern.dir/src/hnsw/external_index.c.o -c
/tmp/lantern/src/hnsw/external_index.c
```
We see
```
-isystem /usr/include/postgresql
```
followed by
```
-isystem /usr/include/postgresql/15/server
```
which means that `PG_VERSION_NUM` would be defined as `160000`.

We see that the first file is from `libpq-dev`, which is not being used
by Lantern.
```
libpq-dev: /usr/include/postgresql/pg_config.h
postgresql-server-dev-15: /usr/include/postgresql/15/server/pg_config.h
```
This patch changes the order of directories for header search to use the
server dir first, so the right `PG_VERSION_NUM` would be defined for
builds for the older PostgreSQL versions.
medvied added a commit to medvied/lantern that referenced this pull request Oct 25, 2023
An issue with PG_VERSION_NUM definition for PostgreSQL < 15 has been
revealed by lanterndata#211: PG_VERSION_NUM is defined for PostgreSQL 16.

Inside the build environment for PostgreSQL 15 it looks like the
following:

```
> grep -R PG_VERSION_NUM /usr/include/
/usr/include/postgresql/pg_config.h:#define PG_VERSION_NUM 160000
/usr/include/postgresql/15/server/pg_config.h:#define PG_VERSION_NUM 150004
/usr/include/postgresql/15/server/fmgr.h:       PG_VERSION_NUM / 100, \
```

And the build command for, for example, `src/hnsw/external_index.c`
looks like this:

```
/usr/bin/cc -DLANTERN_USE_USEARCH -Dlantern_EXPORTS
-I/tmp/lantern/./third_party/usearch/c -isystem /usr/include/postgresql
-isystem /usr/include/postgresql/15/server -isystem /tmp/lantern/src
-isystem /tmp/lantern -Wall -Wextra -Wno-conversion -Wno-unknown-pragmas
-fPIC -MD -MT CMakeFiles/lantern.dir/src/hnsw/external_index.c.o -MF
CMakeFiles/lantern.dir/src/hnsw/external_index.c.o.d -o
CMakeFiles/lantern.dir/src/hnsw/external_index.c.o -c
/tmp/lantern/src/hnsw/external_index.c
```
We see
```
-isystem /usr/include/postgresql
```
followed by
```
-isystem /usr/include/postgresql/15/server
```
which means that `PG_VERSION_NUM` would be defined as `160000`.

We see that the first file is from `libpq-dev`, which is not being used
by Lantern.
```
> dpkg -S /usr/include/postgresql/pg_config.h
libpq-dev: /usr/include/postgresql/pg_config.h
> dpkg -S /usr/include/postgresql/15/server/pg_config.h
postgresql-server-dev-15: /usr/include/postgresql/15/server/pg_config.h
```
This patch changes the order of directories for header search to use the
server dir first, so the right `PG_VERSION_NUM` would be defined for
builds for the older PostgreSQL versions.
Ngalstyan4 pushed a commit that referenced this pull request Oct 25, 2023
An issue with PG_VERSION_NUM definition for PostgreSQL < 15 has been
revealed by #211: PG_VERSION_NUM is defined for PostgreSQL 16.

Inside the build environment for PostgreSQL 15 it looks like the
following:

```
> grep -R PG_VERSION_NUM /usr/include/
/usr/include/postgresql/pg_config.h:#define PG_VERSION_NUM 160000
/usr/include/postgresql/15/server/pg_config.h:#define PG_VERSION_NUM 150004
/usr/include/postgresql/15/server/fmgr.h:       PG_VERSION_NUM / 100, \
```

And the build command for, for example, `src/hnsw/external_index.c`
looks like this:

```
/usr/bin/cc -DLANTERN_USE_USEARCH -Dlantern_EXPORTS
-I/tmp/lantern/./third_party/usearch/c -isystem /usr/include/postgresql
-isystem /usr/include/postgresql/15/server -isystem /tmp/lantern/src
-isystem /tmp/lantern -Wall -Wextra -Wno-conversion -Wno-unknown-pragmas
-fPIC -MD -MT CMakeFiles/lantern.dir/src/hnsw/external_index.c.o -MF
CMakeFiles/lantern.dir/src/hnsw/external_index.c.o.d -o
CMakeFiles/lantern.dir/src/hnsw/external_index.c.o -c
/tmp/lantern/src/hnsw/external_index.c
```
We see
```
-isystem /usr/include/postgresql
```
followed by
```
-isystem /usr/include/postgresql/15/server
```
which means that `PG_VERSION_NUM` would be defined as `160000`.

We see that the first file is from `libpq-dev`, which is not being used
by Lantern.
```
> dpkg -S /usr/include/postgresql/pg_config.h
libpq-dev: /usr/include/postgresql/pg_config.h
> dpkg -S /usr/include/postgresql/15/server/pg_config.h
postgresql-server-dev-15: /usr/include/postgresql/15/server/pg_config.h
```
This patch changes the order of directories for header search to use the
server dir first, so the right `PG_VERSION_NUM` would be defined for
builds for the older PostgreSQL versions.
@medvied
Copy link
Contributor Author

medvied commented Oct 25, 2023

The tests don't fail with #217, so this patch should be merged after #217 is merged.

This patch adds API to trigger execution of C code from SQL to test
corner cases.

`test/sql/hnsw_failure_point.sql` has an example of how to trigger a
process crash using failure points and how to see that a space leak
happens if a crash happens after a block is allocated, but before a
record for the block is added to the index during blockmaps creation.
@medvied medvied marked this pull request as draft October 27, 2023 17:33
@medvied medvied marked this pull request as ready for review November 5, 2023 01:35
@var77
Copy link
Collaborator

var77 commented Dec 11, 2023

Rebased and merged in PR 214

@var77 var77 closed this Dec 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants