Skip to content

Script: Reindex & UpdateByQuery Metadata #88665

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

Merged
merged 25 commits into from
Jul 28, 2022

Conversation

stu-elastic
Copy link
Contributor

Adds metadata classes for Reindex and UpdateByQuery contexts.

For Reindex metadata:

  • _index can't be null
  • _id, _routing and _version are writable and nullable
  • _now is read-only
  • op is read-write must be 'noop', 'index' or 'delete'

Reindex metadata keeps the originx value for _index, _id, _routing and _version
so that Reindexer can see if they've changed.

If _version is null in the ctx map, or, equivalently, the augmentation
setVersionToInternal() was called by the script, Reindexer sets document
versioning to internal. If _version is null in the ctx map, getVersion
returns Long.MIN_VALUE.

For UpdateByQuery metadata:

  • _index, _id, _version, _routing are all read-only
  • _routing is also nullable
  • _now is read-only
  • op is read-write and one of 'index', 'noop', 'delete'

Closes: #86472

Adds metadata classes for Reindex and UpdateByQuery contexts.

For Reindex metadata:
 * _index can't be null
 * _id, _routing and _version are writable and nullable
 * _now is read-only
 * op is read-write must be 'noop', 'index' or 'delete'

Reindex metadata keeps the originx value for _index, _id, _routing and _version
so that `Reindexer` can see if they've changed.

If _version is null in the ctx map, or, equivalently, the augmentation
`setVersionToInternal()` was called by the script, `Reindexer` sets document
versioning to internal.  If `_version` is `null` in the ctx map, `getVersion`
returns `Long.MIN_VALUE`.

For UpdateByQuery metadata:
 * _index, _id, _version, _routing are all read-only
 * _routing is also nullable
 * _now is read-only
 * op is read-write and one of 'index', 'noop', 'delete'

Closes: elastic#86472
@stu-elastic stu-elastic marked this pull request as ready for review July 22, 2022 00:07
@stu-elastic stu-elastic added >enhancement :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache and removed WIP labels Jul 22, 2022
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@elasticsearchmachine elasticsearchmachine added the Team:Core/Infra Meta label for core/infra team label Jul 22, 2022
@elasticsearchmachine
Copy link
Collaborator

Hi @stu-elastic, I've created a changelog YAML for you.

@stu-elastic stu-elastic requested review from jdconrad and rjernst July 22, 2022 14:25
@elasticsearchmachine elasticsearchmachine changed the base branch from master to main July 22, 2022 23:04
Copy link
Contributor

@jdconrad jdconrad left a comment

Choose a reason for hiding this comment

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

@stu-elastic Did a first pass. Looks pretty good. Given that I'm still getting up to speed a bit here, I would've had an easier time if FieldProperty refactorings was a separate PR.

Also please take a look to see if additional validation of source wrapper is indeed a bug fix because if so that will need to be a separate PR. I made a comment in AbstractAsyncBulkByScollAction about this.

Copy link
Contributor

@jdconrad jdconrad left a comment

Choose a reason for hiding this comment

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

LGTM. A couple more minor comments.

public abstract static class ScriptApplier<T extends Metadata>
implements
BiFunction<RequestWrapper<?>, ScrollableHitSource.Hit, RequestWrapper<?>> {
protected static final String INDEX = "index";
Copy link
Contributor

Choose a reason for hiding this comment

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

Would you please add a comment here on what this particular INDEX represents. I believe it's the op, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

public String toString() {
return id.toLowerCase(Locale.ROOT);
}
protected abstract void updateRequest(RequestWrapper<?> request, T metadata);
Copy link
Contributor

Choose a reason for hiding this comment

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

Please move this above requestFromOp just to follow the same pattern as execute.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@mark-vieira mark-vieira added v8.5.0 and removed v8.4.0 labels Jul 27, 2022
Copy link
Contributor

@jdconrad jdconrad left a comment

Choose a reason for hiding this comment

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

Looked again, and still LGTM! Thanks.

@stu-elastic stu-elastic merged commit 476da8c into elastic:main Jul 28, 2022
weizijun added a commit to weizijun/elasticsearch that referenced this pull request Jul 29, 2022
* upstream/main:
  Add 8.5 migration docs (elastic#88923)
  Script: Reindex & UpdateByQuery Metadata (elastic#88665)
  Remove unused plugins dir var from server CLI (elastic#88917)
  Use tracing API in TaskManager (elastic#88885)
  Add source fallback for keyword fields using operation (elastic#88735)
  Prune changelogs after 8.3.3 release
  Bump versions after 8.3.3 release
  Add a test for checking for misspelled "dry_run" parameters for Desired Nodes API (elastic#88898)
  Speedup BalanceUnbalancedClusterTests (elastic#88794)
  Preventing exceptions on node shutdown in integration tests (elastic#88827)
  Do not trigger check part3 for test mute and docs PRs (elastic#88895)
  Add troubleshooting docs about data corruption (elastic#88760)
  Mute RollupActionSingleNodeTests#testRollupDatastream (elastic#88891)
  [DOCS] Domain splitting impacts API keys (elastic#88677)
  Fix SqlSearchIT testAllTypesWithRequestToOldNodes (elastic#88866) (elastic#88883)
  Update synthetic-source.asciidoc (elastic#88880)
  Log more details in TaskAssertions (elastic#88864)
  Make Tuple a record (elastic#88280)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Scripting Scripting abstractions, Painless, and Mustache >enhancement Team:Core/Infra Meta label for core/infra team v8.5.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expose metadata via dedicated class for scripting index and update contexts
4 participants