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

add to version tag to header on producer side #648

Merged
merged 1 commit into from
Nov 10, 2023

Conversation

workeatsleep
Copy link
Contributor

add a tag in header to imply the version

@workeatsleep workeatsleep requested a review from Sunjeet November 8, 2023 00:59
/**
* A header tag indicating which version is this snapshot produce to.
*/
String HEADER_TAG_PRODUCER_TO_VERSION = "hollow.producer.to.version";
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: name hollow.blob.to.version so it is more clear that this is the one in the blob (differentiating the one in metadata)

@@ -388,6 +388,7 @@ long runCycle(
} else {
writeEngine.getHeaderTags().remove(HollowStateEngine.HEADER_TAG_SCHEMA_CHANGE);
}
writeEngine.addHeaderTag(HollowStateEngine.HEADER_TAG_PRODUCER_TO_VERSION, String.valueOf(toVersion));
Copy link
Contributor

@Sunjeet Sunjeet Nov 8, 2023

Choose a reason for hiding this comment

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

The reverse delta header is a special case. When running cycle v2 we want the reverse delta header to have
hollow.producer.to.version=v1
and I think that should be getting handled already here but its worth testing this out and if possible write a unit test, maybe here

@workeatsleep workeatsleep merged commit 7010d51 into master Nov 10, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants