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

Refactor Flint index refresh mode #228

Merged
merged 6 commits into from
Jan 23, 2024

Conversation

dai-chen
Copy link
Collaborator

@dai-chen dai-chen commented Jan 18, 2024

Description

This PR primarily removes the refreshMode in the refreshIndex() API, which is internally used by the SQL layer. This change serves as a prerequisite for follow-up changes that introduce Manual-Incremental refresh mode for #195.

Before the change

Currently, Flint SQL layer always pass the following mode to refresh index API layer:

  • INCREMENTAL mode if CREATE INDEX ... WITH (auto_refresh=true)
  • FULL mode if REFRESH INDEX statement

After the change

The meaning of refresh mode

The revision clarifies that the refresh mode is inherently a characteristic of the Flint index upon creation, rather than something that can be specified later through either the refreshIndex API or the REFRESH statement. In other words, the refreshIndex API will autonomously determine the mode by reading the Flint index options.

Refresh statement behavior

The refresh statement behavior remains unchanged:

  • For manual refresh index, a batch refresh job is triggered as before.
  • For auto refresh index, the refresh statement will exit as before due to the index state already being in the refreshing mode (a new IT has been added to verify this).

Issues Resolved

#100

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.

@dai-chen dai-chen added the maintenance Code refactoring label Jan 18, 2024
@dai-chen dai-chen self-assigned this Jan 18, 2024
@dai-chen dai-chen added the 0.2 label Jan 18, 2024
Comment on lines -346 to -347
case FULL if isIncrementalRefreshing(indexName) =>
throw new IllegalStateException(
s"Index $indexName is incremental refreshing and cannot be manual refreshed")
Copy link
Collaborator Author

@dai-chen dai-chen Jan 18, 2024

Choose a reason for hiding this comment

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

Because refresh mode is decided by index options in refresh index API internally, this case is impossible now.

@dai-chen dai-chen marked this pull request as ready for review January 18, 2024 21:42
Copy link
Collaborator

@noCharger noCharger left a comment

Choose a reason for hiding this comment

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

LGTM

@dai-chen
Copy link
Collaborator Author

Merged from main and resolved conflicts.

@dai-chen dai-chen merged commit d289984 into opensearch-project:main Jan 23, 2024
4 checks passed
@dai-chen dai-chen deleted the remove-refresh-mode branch January 23, 2024 23:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.2 maintenance Code refactoring
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants