-
Notifications
You must be signed in to change notification settings - Fork 25.5k
Replace transport version utils with a build service #133034
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
Conversation
Several transport version build tasks have a need to access the repository-wide transport version resources. Thus far it has been done through several utility methods. This commit moves these utility methods into an encapsulated build service that is shared across the transport version tasks. The advantage is that no paths are used, the build service encapsulates access to the resources and understands internally how to find the correct filesystem path and load it.
Pinging @elastic/es-core-infra (Team:Core/Infra) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Much cleaner.
} | ||
|
||
/** | ||
* Return the transport version resources directory for this repository. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"resources directory" means something specific in this context, but also has a more general meaning that might confuse readers. Should we define this term?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reworded a bit to make clearer these are transport version resources.
.../main/java/org/elasticsearch/gradle/internal/transport/TransportVersionResourcesService.java
Show resolved
Hide resolved
.../main/java/org/elasticsearch/gradle/internal/transport/TransportVersionResourcesService.java
Outdated
Show resolved
Hide resolved
...n/java/org/elasticsearch/gradle/internal/transport/GenerateTransportVersionManifestTask.java
Show resolved
Hide resolved
…p0=search.vectors/45_knn_search_bit/Vector similarity with filter only} elastic#133037
…p0=search.vectors/45_knn_search_bit/Vector rescoring has no effect for non-quantized vectors and provides same results as non-rescored knn} elastic#133039
…33038) With elastic#132774, the overhead of running queries with DOC partitioning is small. While we might switch the default data partitioning to DOC for all queries in the future, this PR defaults data partitioning to DOC for time-series queries only to minimize any unexpected impact. Relates elastic#132774
…tic#133047) Comparing bitsPerOrd == -1 should be a little bit more optimal compared to maxOrd >= 0 (int vs long comparison).
Async queries in EQL and ES|QL do not create an initial response, and the current logic does not correctly handle expiration updates when the query has already completed. With initial response (no change): First, update the expiration in the async index, then update the task's expiration if the task still exists. Without initial response: First, try to update the task's expiration, then attempt to get the result from the task or async index. If the result is no longer available from the task, update the expiration in the async index before retrieving it (similar to the initial response case). This second step was introduced in this fix. Ideally, we should always create the initial response up front to unify the logic for both async_search and async_query, but this fix is preferred for now as it is more contained. When reviewing the code, I also found a race condition where async-get can return a NOT_FOUND error if the task completes but has not yet stored its result in the async index. This issue would also be resolved by storing an initial response up front. I will open a follow-up issue for it. Closes elastic#130619
Replace int hashmap by a counter in TrackingPostingsInMemoryBytesCodec and use int hashset to keep track of seen fields.
…p0=search.highlight/50_synthetic_source/text multi fvh source order} elastic#133056
- add already-tracked DLS cache stats into `/xpack/usage?filter_path=security.roles.dls`
…ndexing {upgradedNodes=1} elastic#133060
…ndexing {upgradedNodes=0} elastic#133061
Pass down actual index version when creating text field mapper builder when mapping dynamic field. Otherwise, bwc logic always has the wrong index version in mixed cluster scenario and dynamic mapping occurs.
@rjernst I think perhaps a merge or rebase went wrong here as there are a ton of unrelated changes in this PR. |
Should be good now |
private Set<String> getMainResources() { | ||
if (mainResources.get() == null) { | ||
synchronized (mainResources) { | ||
String output = gitCommand("ls-tree", "--name-only", "-r", "main", "."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we're going to use relative paths here, then we should explicitly set the working directory when calling exec. There's no real guarantee that this is the workspace directory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We set the working directory in gitCommand
using -C
project.getGradle() | ||
.getSharedServices() | ||
.registerIfAbsent("transportVersionResources", TransportVersionResourcesService.class, spec -> { | ||
Directory transportResources = project.getLayout().getProjectDirectory().dir("src/main/resources/transport"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Presumably this plugin will only ever be applied to a single project, right? Build services are global to the build so all consumers will use the same params. It's not clear if we also use this in serverless and how that will work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, and this being global is part of how it works. Note that this is also used by ValidateTransportVersionReferencesTask which are in every module/plugin. Essentially it's a way to plumb in a wrapper around the resources that are ready from source on disk instead of having shared methods (and the gradle property) to know where to find these resources.
💔 Backport failed
You can use sqren/backport to manually backport by running |
Several transport version build tasks have a need to access the repository-wide transport version resources. Thus far it has been done through several utility methods. This commit moves these utility methods into an encapsulated build service that is shared across the transport version tasks. The advantage is that no paths are used, the build service encapsulates access to the resources and understands internally how to find the correct filesystem path and load it.
This commit backports several changes to the new transport version system relates elastic#133034 relates elastic#133185 relates elastic#133186 relates elastic#133189
This commit backports several changes to the new transport version system relates elastic#133034 relates elastic#133185 relates elastic#133186 relates elastic#133189
This commit backports several changes to the new transport version system relates elastic#133034 relates elastic#133185 relates elastic#133186 relates elastic#133189
* Replace transport version utils with a build service (#133034) Several transport version build tasks have a need to access the repository-wide transport version resources. Thus far it has been done through several utility methods. This commit moves these utility methods into an encapsulated build service that is shared across the transport version tasks. The advantage is that no paths are used, the build service encapsulates access to the resources and understands internally how to find the correct filesystem path and load it. * Make transport version ids have descending ordering (#133185) Transport version ids must be in descending order in definition files. However, the compareTo for them was ascending and the validation used reverse ordering. This commit fixes the compareTo so it uses desending ordering and the validation uses natural ordering. * Fix transport version validation task name (#133186) The task class validates resources, but the task name still had the old "definitions" suffix. * Add separate validation for unreferenced transport version definitions (#133189) This commit splits out the validation that is pertinent to unreferenced definitions. It also adds validation that names of named and unreferenced definitions cannot collide. * Use path separator for git path in transport resources (#133228) When looking up files from main in transport resources, the system dependent path is used. However, the beginning slash also needs to be system dependent.
* Backport several transport version fixes This commit backports several changes to the new transport version system relates #133034 relates #133185 relates #133186 relates #133189 * [CI] Auto commit changes from spotless --------- Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co>
* Backport several transport version fixes This commit backports several changes to the new transport version system relates #133034 relates #133185 relates #133186 relates #133189 * [CI] Auto commit changes from spotless --------- Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co>
Several transport version build tasks have a need to access the repository-wide transport version resources. Thus far it has been done through several utility methods. This commit moves these utility methods into an encapsulated build service that is shared across the transport version tasks. The advantage is that no paths are used, the build service encapsulates access to the resources and understands internally how to find the correct filesystem path and load it.
closes #132788
closes #132789
closes #132790