-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Revert uploading of remote cluster state manifest using min codec version #16403
Revert uploading of remote cluster state manifest using min codec version #16403
Conversation
❌ Gradle check result for 4ea63a5: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
4ea63a5
to
850f6ae
Compare
❌ Gradle check result for 850f6ae: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
850f6ae
to
2349d48
Compare
❌ Gradle check result for 2349d48: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
2349d48
to
5137b75
Compare
❌ Gradle check result for 5137b75: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
bd7249e
to
44c19dd
Compare
44c19dd
to
9835547
Compare
❌ Gradle check result for 9835547: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
9835547
to
fdc4e15
Compare
❌ Gradle check result for fdc4e15: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
fdc4e15
to
e9288b4
Compare
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.
Sorry I am just catching up but based on my limited understanding we need to align codec version bumps with engine versions so that we can be sure we don't back to the older codec after a bump.
We also need to maintain all older codec versions for compatibility, we can remove that logic once there is a migration path of data on older codec versions.
While reading the file we read the codec version from the file name so that we don't need to guess what codec version was used for writing with the guarantee that writes were done in a version not greater than what the reader node can understand
@Bukhtawar We are maintaining the older codec versions for reading reading data written using older codec. Codec version is already there in the filename to determine how to read it. |
❌ Gradle check result for bba8f04: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
bba8f04
to
d534f17
Compare
❌ Gradle check result for d534f17: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Signed-off-by: Sooraj Sinha <[email protected]>
d534f17
to
4a6797c
Compare
@Bukhtawar Merging this PR. This only addresses manifest file and rest of the metadata is also written using latest codec version only. This may need more changes and thinking if we want to provide BWC for write operation as well. |
Signed-off-by: Sooraj Sinha <[email protected]> (cherry picked from commit 4ad1be3) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
(cherry picked from commit 4ad1be3) Signed-off-by: Sooraj Sinha <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Description
When cluster is upgraded from 2.16 or lower to 2.17, the new cluster manager nodes are not able to join the cluster due to deserialization failure while downloading the manifest. The stacktrace is below:
The above stack trace shows that the field
component_prefix
is present in the serialized entity but we do not recognize this field while deserializing. The fieldcomponent_prefix
was added underClusterMetadataManifest.UploadedIndexMetadata
in 2.15 version. In 2.17 version we added an enhancement to support version upgrade: #15216. As part of this change, we upload the manifest using the codec version corresponding to the min node version and download using the same codec version. But forUploadedIndexMetadata
entity in the manifest we are using the latest codec version for uploading due to which thecomponent_prefix
field is always getting serialized which we are not able to deserialize.Fix
Reverting the logic which uploads the manifest using the minimum codec version.
Related Issues
NA
Check List
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.