Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Syntactic indexing: add database schema and dbworker #60055

Merged
merged 48 commits into from
Feb 8, 2024

Conversation

keynmol
Copy link
Contributor

@keynmol keynmol commented Feb 1, 2024

Closes #59794

This PR:

  • Adds a basic schema for syntactic code intel indexes (up and down migration)
  • Sets up everything required to use dbworker interface
  • Adds a stub indexing worker implementation
  • Adds tests to ensure conformance with dbworker interface (run against real postgres)

Test plan

New tests to ensure schema conformance.

For manual testing:

  1. Start the configuration: sg start codeintel-syntactic
  2. Execute this SQL query: insert into syntactic_scip_indexes(id, commit, repository_id, outfile) values (nextval('syntactic_scip_index_id_seq'), '8c0cc755be302107b675824b0fb1b985992092b8', 1, 'index.scip');
  3. Observe the worker pick up the job:
[syntactic-c...1] INFO syntactic-code-intel-worker workerutil/worker.go:329 Dequeued record for processing {"TraceId": "a0b9c4a988d9e46ac42b8c4deb63d670", "SpanId": "6483b7e47fa617f2", "name": "syntactic_code_intel_indexer", "id": "1"}
[syntactic-c...1] INFO syntactic-code-intel-worker.Handle shared/indexing_worker.go:34 Stub indexing worker handling record {"TraceId": "a0b9c4a988d9e46ac42b8c4deb63d670", "SpanId": "470708143297b7ba", "handle": {"id": 1, "repository name": "github.com/sourcegraph-testing/zap", "commit": "8c0cc755be302107b675824b0fb1b985992092b8"}}

@cla-bot cla-bot bot added the cla-signed label Feb 1, 2024
@keynmol keynmol changed the title Move database code out of service entrypoint Batch indexing: add database schema and dbworker Feb 1, 2024
Comment on lines +1262 to +1264
- repo-updater
- gitserver-0
- gitserver-1
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we now require the repo table to be populated for the syntactic_scip_indexes_with_repository_name view to work in joins, it's useful to have repo updater running locally.

@keynmol keynmol marked this pull request as ready for review February 6, 2024 11:15
@keynmol keynmol changed the title Batch indexing: add database schema and dbworker Syntactic indexing: add database schema and dbworker Feb 7, 2024
Copy link
Contributor

@varungandhi-src varungandhi-src left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added some more comments; will do a third pass shortly over some of the SQL.

Copy link
Contributor

@varungandhi-src varungandhi-src left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blocking comments, but some questions and suggestions for improving readability.

Comment on lines +16 to +25
The purpose of this test is to verify that the DB schema
we're using for the syntactic code intel work matches
the requirements of dbworker interface,
and that we can dequeue records through this interface.

The schema is sensitive to column names and types, and to the fact
that we are using a Postgres view to query repository name alongside
indexing records,
so it's important that we use the real Postgres in this test to prevent
schema/implementation drift.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding this comment, it's very useful

@keynmol
Copy link
Contributor Author

keynmol commented Feb 8, 2024

@varungandhi-src once the build is green I will merge this - if you have any more comments, please add them here or raise a separate issue.

@keynmol keynmol merged commit d5f5337 into main Feb 8, 2024
@keynmol keynmol deleted the syntactic-worker-database branch February 8, 2024 18:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Syntactic indexing: add DB migration for tables and views required by dbworker
2 participants