search

lanterndata / lantern

PostgreSQL vector database extension for building AI applications
https://lantern.dev
GNU Affero General Public License v3.0
790 stars 56 forks source link

Stop postgres using pg_ctl to avoid race conditions #257

Closed var77 closed 10 months ago

var77 commented 10 months ago

There were 2 issues:

  1. pg_isready was getting permissions issue when running from /home/runner/work/lantern/lantern (pg_response could not change directory to "/home/runner/work/lantern/lantern": Permission denied) . So I am changing directory to /tmp before running it.
  2. When just killing postgres process via PID it needs some time before completely shut down. So if we kill postgres and immediately run again, the run might fail because some filesystem locks. Now we will stop postgres using pg_ctl which will wait for the process to completely stop before returning
github-actions[bot] commented 10 months ago

Benchmarks

metric old new pct change
recall (after create) 0.740 0.740 -
recall (after insert) 0.754 0.772 +2.39%
select bulk tps 475.334 481.754 +1.35%
select bulk latency (ms) 16.072 15.884 -1.17%
select bulk latency (stddev ms) 2.455 3.537 +44.07%
create latency (ms) 1241.714 1216.585 -2.02%
insert bulk tps 10.426 10.403 -0.22%
insert bulk latency (ms) 95.905 96.118 +0.22%
insert bulk latency (stddev ms) 3.760 3.945 +4.92%
disk usage (bytes) 6348800.000 6348800.000 -
codecov[bot] commented 10 months ago

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (9ac519e) 81.25% compared to head (596c671) 81.80%. Report is 1 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #257 +/- ## ========================================== + Coverage 81.25% 81.80% +0.54% ========================================== Files 17 17 Lines 1478 1533 +55 Branches 338 345 +7 ========================================== + Hits 1201 1254 +53 + Misses 126 124 -2 - Partials 151 155 +4 ``` | [Files](https://app.codecov.io/gh/lanterndata/lantern/pull/257?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lanterndata) | Coverage Δ | | |---|---|---| | [src/hnsw/extra\_dirtied.c](https://app.codecov.io/gh/lanterndata/lantern/pull/257?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lanterndata#diff-c3JjL2huc3cvZXh0cmFfZGlydGllZC5j) | `91.66% <100.00%> (+1.10%)` | :arrow_up: | | [src/hnsw/build.c](https://app.codecov.io/gh/lanterndata/lantern/pull/257?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lanterndata#diff-c3JjL2huc3cvYnVpbGQuYw==) | `81.41% <95.45%> (+1.73%)` | :arrow_up: | | [src/hnsw/insert.c](https://app.codecov.io/gh/lanterndata/lantern/pull/257?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lanterndata#diff-c3JjL2huc3cvaW5zZXJ0LmM=) | `79.41% <80.00%> (-0.59%)` | :arrow_down: | | [src/hnsw/external\_index.c](https://app.codecov.io/gh/lanterndata/lantern/pull/257?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lanterndata#diff-c3JjL2huc3cvZXh0ZXJuYWxfaW5kZXguYw==) | `90.59% <90.62%> (+0.14%)` | :arrow_up: |
var77 commented 10 months ago

I think for now it should not fail, I will try to remove || true's and see. It will fail if there won't be postgres process running but we should not have that case