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

feat: prepare embedder for Production #9517

Merged
merged 44 commits into from
Mar 29, 2024
Merged

feat: prepare embedder for Production #9517

merged 44 commits into from
Mar 29, 2024

Conversation

mattkrick
Copy link
Member

@mattkrick mattkrick commented Mar 8, 2024

Description

Refactors the embedder to handle production workload.

Out of scope

  • detect when there's not a enough data, or the data is not english
  • consider adversarial models to grade a set of teamIds
  • handle priority (e.g. process on feature flag or teamId, etc)

Demo
https://www.loom.com/share/e2d9d43d6d924676bb8a3cce84edb336?sid=82842bf2-68c5-42e5-b4f5-3c7f482b8e0b

How it works:

  • 1 worker from the cluster becomes primary and builds the tables, triggers, and historical metadata
  • when a meeting ends, the gql executor sends a message to the embedder via redis streams so only 1 worker gets the job to add the discussion to metadata table
  • When a row is inserted into the metadata table, it also gets added to the job queue table. This happens via trigger. there is 1 trigger per embedding model
  • The embedder polls the PG job queue table to get a job. it creates fullText and embeds that. it then writes that embedding to the model's table.
  • Can add items to the metadata by adhoc query passing in startAt and/or endAt
  • every 5 mins the primary worker resets stalled embedding jobs

Signed-off-by: Matt Krick <matt.krick@gmail.com>
Signed-off-by: Matt Krick <matt.krick@gmail.com>
Signed-off-by: Matt Krick <matt.krick@gmail.com>
Copy link
Contributor

github-actions bot commented Mar 8, 2024

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR will be delayed and might be rejected due to its size.

"id" INT GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY,
"embedText" TEXT,
"embedding" vector(${sql.raw(vectorDimensions.toString())}),
"embeddingsMetadataId" INTEGER UNIQUE NOT NULL,
Copy link
Member Author

Choose a reason for hiding this comment

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

@jordanh safe to add the unique constraint here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I think that is ok. It would imply that if we ever have multiple versions of the same embedding models, they'll need to live on their own table which I think is what we'd want

Signed-off-by: Matt Krick <matt.krick@gmail.com>
Copy link
Contributor

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR will be delayed and might be rejected due to its size.

Signed-off-by: Matt Krick <matt.krick@gmail.com>
Copy link
Contributor

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR will be delayed and might be rejected due to its size.

Signed-off-by: Matt Krick <matt.krick@gmail.com>
Signed-off-by: Matt Krick <matt.krick@gmail.com>
Signed-off-by: Matt Krick <matt.krick@gmail.com>
Copy link
Contributor

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR will be delayed and might be rejected due to its size.

Signed-off-by: Matt Krick <matt.krick@gmail.com>
Copy link
Contributor

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR will be delayed and might be rejected due to its size.

Copy link
Contributor

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR will be delayed and might be rejected due to its size.

Copy link
Contributor

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR will be delayed and might be rejected due to its size.

Signed-off-by: Matt Krick <matt.krick@gmail.com>
Copy link
Contributor

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR will be delayed and might be rejected due to its size.

Signed-off-by: Matt Krick <matt.krick@gmail.com>
Copy link
Contributor

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR will be delayed and might be rejected due to its size.

Copy link
Contributor

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR will be delayed and might be rejected due to its size.

@rafaelromcar-parabol
Copy link
Contributor

Updating a readme whenever the version changes isn't great. While looking over that readme again, the build section is basically just build.yml in markdown, right? Can we remove that section & just say refer to the build.yml to see how to build it?

PR #9558

I answered the PR. As it is a different PR and we will be discussing it over there, I suggest you do the changes I proposed, just to be consistent with the current state of things, and we then see what to do with the future.

Signed-off-by: Matt Krick <matt.krick@gmail.com>
Copy link
Contributor

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR will be delayed and might be rejected due to its size.

Signed-off-by: Matt Krick <matt.krick@gmail.com>
Copy link
Contributor

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR will be delayed and might be rejected due to its size.

Signed-off-by: Matt Krick <matt.krick@gmail.com>
Copy link
Contributor

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR will be delayed and might be rejected due to its size.

Signed-off-by: Matt Krick <matt.krick@gmail.com>
Copy link
Contributor

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR will be delayed and might be rejected due to its size.

Signed-off-by: Matt Krick <matt.krick@gmail.com>
Copy link
Contributor

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR will be delayed and might be rejected due to its size.

Signed-off-by: Matt Krick <matt.krick@gmail.com>
Copy link
Contributor

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR will be delayed and might be rejected due to its size.

Signed-off-by: Matt Krick <matt.krick@gmail.com>
Copy link
Contributor

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR will be delayed and might be rejected due to its size.

Signed-off-by: Matt Krick <matt.krick@gmail.com>
Copy link
Contributor

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR will be delayed and might be rejected due to its size.

Copy link
Contributor

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR will be delayed and might be rejected due to its size.

1 similar comment
Copy link
Contributor

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR will be delayed and might be rejected due to its size.

Signed-off-by: Matt Krick <matt.krick@gmail.com>
Copy link
Contributor

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR will be delayed and might be rejected due to its size.

@mattkrick mattkrick merged commit 538c95c into master Mar 29, 2024
6 checks passed
@mattkrick mattkrick deleted the feat/embedder1 branch March 29, 2024 00:19
@github-actions github-actions bot mentioned this pull request Apr 1, 2024
24 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants