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

Switch to explicit index management #41

Conversation

danielmitterdorfer
Copy link
Member

With this commit we switch all tracks to explicit index management
(instead of index auto-management). As this change is breaking, we also
increase the track version from 1 to 2 thus requiring at least Rally
0.9.0 to use these tracks.

This allows us to remove also some legacy code that has registered
the refresh operation for older versions of Rally (newer versions
already include this operation in Rally core).

Relates elastic/rally#380
Closes #37

With this commit we switch all tracks to explicit index management
(instead of index auto-management). As this change is breaking, we also
increase the track version from 1 to 2 thus requiring at least Rally
0.9.0 to use these tracks.
This allows us to also remove some legacy codd that has registered the
`refresh` operation for older versions of Rally (newer versions already
include this operation in Rally core).

Relates elastic/rally#380
Closes elastic#37
Copy link
Contributor

@dliappis dliappis left a comment

Choose a reason for hiding this comment

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

Great work! So much de-duplication here, only the removal of mappings.json by itself is worth the effort (and the ensuing confusion for newcomers about why the same is present in index.json :) )

LGTM, Bravo.

"uncompressed-bytes": 23256051757
}
]
"types": ["articles"]
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 Apart from becoming mandatory in 0.10 it also helps beginners, who typically use our tracks for inspiration to write their first track, to see our track definitions match the documentation :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Also the removal of mappings.json will help; in the beginning I was also wondering why the definitions were in two places, both this file and the "mappings" object in index.json :)

Copy link
Member Author

Choose a reason for hiding this comment

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

That was the necessary evil of backwards-compatibility. :)

@@ -99,33 +118,4 @@
"target-throughput": 50
}
]
},
{
"name": "append-no-conflicts-index-only",
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't need this challenge any more? I would have assumed we'd just need to replace the operations here with a delete / create / index, but what am I missing?

Copy link
Member Author

Choose a reason for hiding this comment

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

Percolator really is about querying, indexing is just a means to an end here and I figured we can get rid of it right away. If somebody really wants this (which they shouldn't), then they can just use task filtering to get it back.

@danielmitterdorfer danielmitterdorfer merged commit 6477977 into elastic:master Apr 6, 2018
danielmitterdorfer added a commit to elastic/rally that referenced this pull request Apr 6, 2018
We disable integration tests for older versions of ES temporarily as we
need to backport changes from elastic/rally-tracks#41 first.
danielmitterdorfer added a commit that referenced this pull request Apr 6, 2018
With this commit we switch all tracks to explicit index management
(instead of index auto-management). As this change is breaking, we also
increase the track version from 1 to 2 thus requiring at least Rally
0.9.0 to use these tracks.
This allows us to also remove some legacy codd that has registered the
`refresh` operation for older versions of Rally (newer versions already
include this operation in Rally core).

Relates elastic/rally#380
Closes #37
Relates #41
danielmitterdorfer added a commit that referenced this pull request Apr 6, 2018
With this commit we switch all tracks to explicit index management
(instead of index auto-management). As this change is breaking, we also
increase the track version from 1 to 2 thus requiring at least Rally
0.9.0 to use these tracks.
This allows us to also remove some legacy codd that has registered the
`refresh` operation for older versions of Rally (newer versions already
include this operation in Rally core).

Relates elastic/rally#380
Closes #37
Relates #41
danielmitterdorfer added a commit that referenced this pull request Apr 6, 2018
With this commit we switch all tracks to explicit index management
(instead of index auto-management). As this change is breaking, we also
increase the track version from 1 to 2 thus requiring at least Rally
0.9.0 to use these tracks.
This allows us to also remove some legacy codd that has registered the
`refresh` operation for older versions of Rally (newer versions already
include this operation in Rally core).

Relates elastic/rally#380
Closes #37
Relates #41
@danielmitterdorfer danielmitterdorfer deleted the explicit-index-management branch April 6, 2018 10:45
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.

2 participants