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

Index routing tool #110

Draft
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

Hailong-am
Copy link
Contributor

Description

use this tool to select appropriate index for PPL query

Issues Resolved

[List any issues this PR will resolve]

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Copy link

codecov bot commented Jan 5, 2024

Codecov Report

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

Comparison is base (7a596a0) 81.70% compared to head (4d41090) 50.58%.
Report is 13 commits behind head on main.

❗ Current head 4d41090 differs from pull request most recent head 7ede795. Consider uploading reports for the commit 7ede795 to get more accurate results

Files Patch % Lines
...opensearch/agent/job/IndexSummaryEmbeddingJob.java 0.00% 185 Missing ⚠️
...a/org/opensearch/agent/tools/IndexRoutingTool.java 0.00% 132 Missing ⚠️
...va/org/opensearch/agent/indices/IndicesHelper.java 0.00% 96 Missing ⚠️
.../main/java/org/opensearch/agent/job/MLClients.java 0.00% 79 Missing ⚠️
...h/agent/job/SkillsClusterManagerEventListener.java 0.00% 68 Missing ⚠️
...java/org/opensearch/agent/job/AgentMonitorJob.java 0.00% 22 Missing ⚠️
src/main/java/org/opensearch/agent/ToolPlugin.java 0.00% 16 Missing ⚠️
.../org/opensearch/agent/tools/utils/LLMProvider.java 0.00% 11 Missing ⚠️
.../opensearch/agent/tools/AbstractRetrieverTool.java 84.21% 1 Missing and 2 partials ⚠️
.../org/opensearch/agent/indices/SkillsIndexEnum.java 0.00% 2 Missing ⚠️
... and 1 more
Additional details and impacted files
@@              Coverage Diff              @@
##               main     #110       +/-   ##
=============================================
- Coverage     81.70%   50.58%   -31.13%     
- Complexity      190      197        +7     
=============================================
  Files            13       22        +9     
  Lines           962     1627      +665     
  Branches        130      189       +59     
=============================================
+ Hits            786      823       +37     
- Misses          126      752      +626     
- Partials         50       52        +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Hailong Cui <ihailong@amazon.com>

job

Signed-off-by: Hailong Cui <ihailong@amazon.com>

remove job scheduler dependnecies

Signed-off-by: Hailong Cui <ihailong@amazon.com>

index summary job

Signed-off-by: Hailong Cui <ihailong@amazon.com>
Signed-off-by: Hailong Cui <ihailong@amazon.com>
Signed-off-by: Hailong Cui <ihailong@amazon.com>
Signed-off-by: Hailong Cui <ihailong@amazon.com>
Signed-off-by: Hailong Cui <ihailong@amazon.com>
Signed-off-by: Hailong Cui <ihailong@amazon.com>

// call LLM, MLModelTool
String question = parameters.get(INPUT_FIELD);
String prompt = buildPrompt(summaryStr, question);

Choose a reason for hiding this comment

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

We might want to guard the number of tokens in the summaryStr, when it is too long, the performance of Claude-instant drops significantly and it is expensive. (I have some idea/implementation in Python, we can discuss how to best proceed there.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right, that's in todo list. let's have a discuss about how to implement it.

Signed-off-by: Hailong Cui <ihailong@amazon.com>
@Hailong-am Hailong-am changed the title [WIP] Index routing tool Index routing tool Jan 11, 2024
@Hailong-am Hailong-am marked this pull request as ready for review January 11, 2024 08:51
Signed-off-by: Hailong Cui <ihailong@amazon.com>
public static String INDEX_SUMMARY = "summary";
public static String INDEX_EMBEDDING = "embedding";
public static String SENTENCE_EMBEDDING = "sentence_embedding";
public static int DEFAULT_TIMEOUT_SECOND = 30;

Choose a reason for hiding this comment

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

Just FYI, when I tested it locally on my mac, with 60-70 indexes, 30 seconds seems not enough.

Signed-off-by: Hailong Cui <ihailong@amazon.com>
@@ -0,0 +1,19 @@
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's this new index for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this index served as vector store for index summary embeddings.

import lombok.extern.log4j.Log4j2;

/**
* monitor agent change and trigger index summary embedding job for new agent
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why need to add this job?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IndexSummaryEmbedding job run with less frequency like daily, and AgentMonitorJob rum more frequently to run for newly created agent which has IndexRoutingTool configured.

Signed-off-by: Hailong Cui <ihailong@amazon.com>
Signed-off-by: Hailong Cui <ihailong@amazon.com>
Signed-off-by: Hailong Cui <ihailong@amazon.com>
Signed-off-by: Hailong Cui <ihailong@amazon.com>
Signed-off-by: Hailong Cui <ihailong@amazon.com>
Signed-off-by: Hailong Cui <ihailong@amazon.com>
Signed-off-by: Hailong Cui <ihailong@amazon.com>
@Hailong-am Hailong-am marked this pull request as draft February 2, 2024 05:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants