External indexing via tcp socket by var77 · Pull Request #324 · lanterndata/lantern

var77 · 2024-07-16T09:13:51Z

add external param to index creation
add lantern_hnsw.external_index_host GUC.
add lantern_hnsw.external_index_port GUC.

When the external parameter will be passed we connect to remote lantern daemon via tcp socket using the external_index_host and external_index_port GUC variables. First we send the index header to socket and codebook if needed. After getting ack response we start streaming tuples via socket to daemon which will create multithreaded usearch index. Then it will send back the num_tuples_added and index_size (which is the file size for index) and start streaming index file. We will collect index file into result_buf and use that to write the index into disk. For now we assume that the data sent will be in little endian byte ordering.

Corresponding PR in Lantern Extras PR

TODO

add tests

github-actions · 2024-07-16T09:22:28Z

Benchmarks

metric	old	new	pct change
recall (after create)	0.954	0.952	-0.21%
recall (after insert)	0.000	0.000	-
select tps	12226.184	12513.687	+2.35%
select bulk(100) tps	32.354	36.207	+11.91%
select latency (ms)	0.756 ± 2.277𝜎	0.763 ± 1.962𝜎	+0.93%
select bulk(100) latency (ms)	922.465 ± 149.265𝜎	857.631 ± 113.802𝜎	-7.03%
create latency (ms)	396700.063	394004.141	-0.68%
insert tps	455.562	450.699	-1.07%
insert bulk(100) tps	4.873	4.688	-3.79%
insert latency (ms)	69.561 ± 16.001𝜎	70.363 ± 16.073𝜎	+1.15%
insert bulk(100) latency (ms)	6443.522 ± 116.431𝜎	6682.611 ± 187.341𝜎	+3.71%
disk usage (bytes)	8192008192.000	8192008192.000	-

Add `lantern_hnsw.external_index_host` GUC. Add `lantern_hnsw.external_index_port` GUC. When the `external` parameter will be passed we connect to remote lantern daemon via tcp socket using the `external_index_host` and `external_index_port` GUC variables. First we send the index header to socket and codebook if needed. After getting ack response we start streaming tuples via socket to daemon which will create multithreaded usearch index. Then it will send back the `num_tuples_added` and `index_size` (which is the file size for index) and start streaming index file. We will collect index file into `result_buf` and use that to write the index into disk. For now we assume that the data sent will be in little endian byte ordering.

codecov · 2024-07-16T09:30:41Z

Codecov Report

Attention: Patch coverage is 11.53846% with 161 lines in your changes missing coverage. Please review.

Files	Patch %	Lines
src/hnsw/external_index_socket.c	0.00%	144 Missing ⚠️
src/hnsw/build.c	39.28%	5 Missing and 12 partials ⚠️

📢 Thoughts on this report? Let us know!

greptile-apps

PR Summary

Added new fixtures and tests for external indexing via TCP socket in /scripts/integration_tests.py
Introduced GUC variables lantern_hnsw.external_index_host and lantern_hnsw.external_index_port in /src/hnsw/options.c
Added external parameter for index creation in /src/hnsw/options.c
Updated ldb_HnswOptions struct with external parameter and added ldb_HnswGetExternal function in /src/hnsw/options.h
Enhanced scalability and performance by enabling connection to remote Lantern daemon for multithreaded index creation

_{3 file(s) reviewed, 13 comment(s)}
_{Edit PR Review Bot Settings}

greptile-apps · 2024-07-17T14:51:35Z

scripts/integration_tests.py

+import socket
+import subprocess
+import time


Info: Added imports for socket, subprocess, and time to support external indexing setup.

greptile-apps · 2024-07-17T14:51:36Z

scripts/integration_tests.py

+# fixture to handle external index server setup
+@pytest.fixture
+def external_index(request):
+    cli_path = os.getenv("LANTERN_CLI_PATH")
+    if not cli_path:
+        pytest.skip("pass 'LANTERN_CLI_PATH' environment variable to run external indexing tests")
+        return
+
+    with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s:
+        if s.connect_ex(('127.0.0.1', 8998)) != 0:
+            subprocess.Popen([cli_path, "start-indexing-server", "--host", "127.0.0.1", "--port", "8998"], shell=False,
+             stdin=None, stdout=None, stderr=None, close_fds=True)
+            tries = 1
+            time.sleep(5)
+


Info: Added fixture external_index to handle the setup of the external index server.

greptile-apps · 2024-07-17T14:51:37Z

scripts/integration_tests.py

+            tries = 1
+            time.sleep(5)
+
+@pytest.mark.parametrize("distance_metric", ["l2sq", "cos"])


Info: Added test test_external_index to create and validate an external index using the new external parameter.

greptile-apps · 2024-07-17T14:51:38Z

scripts/integration_tests.py

+    primary.execute("testdb", f"SELECT _lantern_internal.validate_index('{index_name}')")
+
+
+@pytest.mark.external_index


Info: Added test test_external_index_pq to create and validate an external index with product quantization (pq) enabled.

greptile-apps · 2024-07-17T14:52:27Z

src/hnsw/options.c

+int   ldb_external_index_port;
+char *ldb_external_index_host;


Info: Added new GUC variables ldb_external_index_port and ldb_external_index_host for external indexing configuration.

src/hnsw/options.c

greptile-apps · 2024-07-17T14:52:31Z

src/hnsw/options.c

+    DefineCustomStringVariable("lantern_hnsw.external_index_host",
+                               "Host for external indexing",
+                               "Change this value if you run lantern daemon on remote host",
+                               &ldb_external_index_host,
+                               "127.0.0.1",
+                               PGC_USERSET,
+                               0,
+                               NULL,
+                               NULL,
+                               NULL);


Info: Defined lantern_hnsw.external_index_host GUC variable for specifying the host used for external indexing.

greptile-apps · 2024-07-17T14:53:05Z

src/hnsw/options.h

    int   ef_construction;
    int   ef;
    bool  pq;
+    bool  external;


Info: Added external parameter to support external indexing via TCP socket.

greptile-apps · 2024-07-17T14:53:05Z

src/hnsw/options.h

 int                   ldb_HnswGetEf(Relation index);
 char*                 ldb_HnswGetIndexFilePath(Relation index);
 bool                  ldb_HnswGetPq(Relation index);
+bool                  ldb_HnswGetExternal(Relation index);


Info: Added getter function for the new external parameter.

greptile-apps · 2024-07-17T14:53:06Z

src/hnsw/options.h

+extern int   ldb_external_index_port;
+extern char* ldb_external_index_host;


Info: Introduced new GUC variables for external index host and port.

fix header size issue in external indexing

Ngalstyan4 · 2024-07-18T06:36:28Z

ci/scripts/build-linux.sh

+function setup_rust() {
+  curl -k -o /tmp/rustup.sh https://sh.rustup.rs
+  chmod +x /tmp/rustup.sh
+  /tmp/rustup.sh -y


Do we need a specific version of rust or this is fine ?

Ngalstyan4 · 2024-07-18T06:39:38Z

scripts/integration_tests.py

+    # note: when M is too low, the streaming search below might return fewer results since the bottom layer of hnsw is less connected
+    primary.execute(
+        "testdb",
+        f"CREATE INDEX {index_name} ON {table_name} USING lantern_hnsw (v {ops}) WITH (dim=128, M=10, quant_bits = {quant_bits}, external = true)",


Is the test set up such that this gets executed for various quant bits or does this only run once ?

yes this gets executed for 8, 16, 32 quant bits and cos, l2sq distances

Ngalstyan4 · 2024-07-18T06:55:26Z

src/hnsw/build.c

+{
+    if(bytes_written > 0) return;
+
+    shutdown(client_fd, SHUT_RDWR);


Why shutdown if you are closing right after?
Is this necessary ?

Ngalstyan4 · 2024-07-18T06:59:45Z

src/hnsw/build.c

+            client_fd, buildstate->pq_codebook, params->dimensions, params->num_centroids, params->num_subvectors);
+    }
+
+    uint32 buf_size = read(client_fd, &init_response, 1024);


Make 1024 a named constant here and at chat array definition

Ngalstyan4 · 2024-07-18T07:02:28Z

src/hnsw/build.c

-        usearch_reserve(buildstate->usearch_index, estimated_row_count, &error);
+
+        if(buildstate->external) {
+            assert(is_little_endian());


Make this "if" and elog error on fail, so the check happens in release builds

Ngalstyan4 · 2024-07-18T07:04:26Z

src/hnsw/build.c

+    if(!buildstate->external) {
+        fstat(index_file_fd, &index_file_stat);
+        result_buf = mmap(NULL, index_file_stat.st_size, PROT_READ, MAP_PRIVATE, index_file_fd, 0);
+        assert(result_buf != MAP_FAILED);


Make this elog error
Q: is it safe to elog error from this context ?

Ngalstyan4 · 2024-07-18T07:37:25Z

src/hnsw/build.c

+            bytes_read = read(buildstate->external_client_fd, result_buf + total_received, BUFFER_SIZE);
+
+            // Check for CTRL-C interrupts
+            CHECK_FOR_INTERRUPTS();


try or test this

…socket, close socket on interrupt

…rray on external index

Ngalstyan4 · 2024-07-19T15:21:40Z

src/hnsw/build.c

        fstat(index_file_fd, &index_file_stat);
        result_buf = mmap(NULL, index_file_stat.st_size, PROT_READ, MAP_PRIVATE, index_file_fd, 0);
-        assert(result_buf != MAP_FAILED);
+        if(result_buf == MAP_FAILED) {


is this the only failure condition?
does mmap API guarantee that if this branch is not taken mmap succeeded?

Ngalstyan4 · 2024-07-25T07:55:10Z

src/hnsw/external_index_socket.c

    }
 }

+static int connect_with_timeout(int sockfd, const struct sockaddr *addr, socklen_t addrlen, int timeout)


Write a comment here so we do not forget what this is about. something like:

This function ensures we do not get stuck connecting to an unavailable endpoint.
It creates a non-blocking socket, then converts the socket to a blocking one after the connection is established

…result buffer in case of external indexing

var77 force-pushed the varik/streaming-external-indexing branch from 8f64a61 to a6b6dca Compare July 16, 2024 09:23

var77 force-pushed the varik/streaming-external-indexing branch from eb58fbc to a1efe8a Compare July 17, 2024 14:49

var77 marked this pull request as ready for review July 17, 2024 14:51

var77 requested a review from Ngalstyan4 July 17, 2024 14:51

greptile-apps bot reviewed Jul 17, 2024

View reviewed changes

var77 force-pushed the varik/streaming-external-indexing branch from a1efe8a to e2c459d Compare July 17, 2024 14:57

add integration tests for external indexing

c506a37

fix header size issue in external indexing

var77 force-pushed the varik/streaming-external-indexing branch from e2c459d to c506a37 Compare July 17, 2024 15:07

Ngalstyan4 reviewed Jul 18, 2024

View reviewed changes

move external indexing to separate file, add read/write timeouts for …

ac412bb

…socket, close socket on interrupt

var77 mentioned this pull request Jul 18, 2024

Call external index function and import created index file on BuildIndex if external=true #298

Closed

var77 added 3 commits July 19, 2024 20:18

add connect timeout, check for interrupts manually

4c2103b

fix index dimensions for hamming distance, set correct size for bit a…

44e8ac5

…rray on external index

update version to v0.3.2

29c2cd9

var77 requested a review from Ngalstyan4 July 23, 2024 15:09

Ngalstyan4 reviewed Jul 25, 2024

View reviewed changes

var77 added 2 commits July 25, 2024 12:26

add comments explaining connect_with_timeout function, do not munmup …

dc5447e

…result buffer in case of external indexing

change lantern cli branch to main in CI job

627d950

var77 merged commit 84cb0f6 into main Jul 25, 2024

var77 deleted the varik/streaming-external-indexing branch July 25, 2024 11:42

		primary.execute("testdb", f"SELECT _lantern_internal.validate_index('{index_name}')")


		@pytest.mark.external_index

		extern int ldb_external_index_port;
		extern char* ldb_external_index_host;

Conversation

var77 commented Jul 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

TODO

Uh oh!

github-actions bot commented Jul 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmarks

Uh oh!

codecov bot commented Jul 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

Uh oh!

greptile-apps bot Jul 17, 2024

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jul 17, 2024

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jul 17, 2024

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jul 17, 2024

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jul 17, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

greptile-apps bot Jul 17, 2024

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jul 17, 2024

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jul 17, 2024

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jul 17, 2024

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

var77 commented Jul 16, 2024 •

edited

Loading

github-actions bot commented Jul 16, 2024 •

edited

Loading

codecov bot commented Jul 16, 2024 •

edited

Loading