-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Fix some lazy rollover code #124153
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
Fix some lazy rollover code #124153
Conversation
The main goal of the PR is to fix the "rollover failed" log in the `IndexTemplateRegistry`. We were logging this on every upgrade because `rolloverResponse.isRolledOver()` is always `false` when we do a lazy rollover request - which is the default for index template updates. While I was looking at this, I noticed we don't actually return early when we process a lazy rollover _dry run_. The end result happened to be ok (i.e. side-effects and response), but I figured I might as well fix the behavior to return early. Finally, we add an `INFO` log in the lazy rollover action to aid the debugging process.
|
Pinging @elastic/es-data-management (Team:Data Management) |
gmarouli
left a comment
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.
Nice catch! Thank you for fixing it, I left some minor comments but LGTM.
.../test/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverActionTests.java
Show resolved
Hide resolved
...k/plugin/core/src/main/java/org/elasticsearch/xpack/core/template/IndexTemplateRegistry.java
Outdated
Show resolved
Hide resolved
gmarouli
left a comment
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.
LGTM 🚀 ship it :)
💔 Backport failed
You can use sqren/backport to manually backport by running |
The main goal of the PR is to fix the "rollover failed" log in the `IndexTemplateRegistry`. We were logging this on every upgrade because `rolloverResponse.isRolledOver()` is always `false` when we do a lazy rollover request - which is the default for index template updates. While I was looking at this, I noticed we don't actually return early when we process a lazy rollover _dry run_. The end result happened to be ok (i.e. side-effects and response), but I figured I might as well fix the behavior to return early. Finally, we add an `INFO` log in the lazy rollover action to aid the debugging process. (cherry picked from commit 9544204) # Conflicts: # server/src/main/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverAction.java
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation |
The main goal of the PR is to fix the "rollover failed" log in the `IndexTemplateRegistry`. We were logging this on every upgrade because `rolloverResponse.isRolledOver()` is always `false` when we do a lazy rollover request - which is the default for index template updates. While I was looking at this, I noticed we don't actually return early when we process a lazy rollover _dry run_. The end result happened to be ok (i.e. side-effects and response), but I figured I might as well fix the behavior to return early. Finally, we add an `INFO` log in the lazy rollover action to aid the debugging process. (cherry picked from commit 9544204) # Conflicts: # server/src/main/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverAction.java
The main goal of the PR is to fix the "rollover failed" log in the `IndexTemplateRegistry`. We were logging this on every upgrade because `rolloverResponse.isRolledOver()` is always `false` when we do a lazy rollover request - which is the default for index template updates. While I was looking at this, I noticed we don't actually return early when we process a lazy rollover _dry run_. The end result happened to be ok (i.e. side-effects and response), but I figured I might as well fix the behavior to return early. Finally, we add an `INFO` log in the lazy rollover action to aid the debugging process. (cherry picked from commit 9544204) # Conflicts: # server/src/main/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverAction.java
The main goal of the PR is to fix the "rollover failed" log in the `IndexTemplateRegistry`. We were logging this on every upgrade because `rolloverResponse.isRolledOver()` is always `false` when we do a lazy rollover request - which is the default for index template updates. While I was looking at this, I noticed we don't actually return early when we process a lazy rollover _dry run_. The end result happened to be ok (i.e. side-effects and response), but I figured I might as well fix the behavior to return early. Finally, we add an `INFO` log in the lazy rollover action to aid the debugging process. (cherry picked from commit 9544204) # Conflicts: # server/src/main/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverAction.java
The main goal of the PR is to fix the "rollover failed" log in the `IndexTemplateRegistry`. We were logging this on every upgrade because `rolloverResponse.isRolledOver()` is always `false` when we do a lazy rollover request - which is the default for index template updates. While I was looking at this, I noticed we don't actually return early when we process a lazy rollover _dry run_. The end result happened to be ok (i.e. side-effects and response), but I figured I might as well fix the behavior to return early. Finally, we add an `INFO` log in the lazy rollover action to aid the debugging process. (cherry picked from commit 9544204) # Conflicts: # server/src/main/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverAction.java
The main goal of the PR is to fix the "rollover failed" log in the `IndexTemplateRegistry`. We were logging this on every upgrade because `rolloverResponse.isRolledOver()` is always `false` when we do a lazy rollover request - which is the default for index template updates. While I was looking at this, I noticed we don't actually return early when we process a lazy rollover _dry run_. The end result happened to be ok (i.e. side-effects and response), but I figured I might as well fix the behavior to return early. Finally, we add an `INFO` log in the lazy rollover action to aid the debugging process.
The main goal of the PR is to fix the "rollover failed" log in the
IndexTemplateRegistry. We were logging this on every upgrade becauserolloverResponse.isRolledOver()is alwaysfalsewhen we do a lazy rollover request - which is the default for index template updates.While I was looking at this, I noticed we don't actually return early when we process a lazy rollover dry run. The end result happened to be ok (i.e. side-effects and response), but I figured I might as well fix the behavior to return early.
Finally, we add an
INFOlog in the lazy rollover action to aid the debugging process.