change consumer to read metadata in blob header for version number #650
Conversation
| long expectedToVersion = transition.getToVersion(); | ||
| Long actualToVersion = null; | ||
| String actualToVersionStr = stateEngine.getHeaderTag(HEADER_TAG_PRODUCER_TO_VERSION); | ||
| if (actualToVersionStr != null) { |
There was a problem hiding this comment.
nit: can collapse the two if blocks that are checking the same condition
There was a problem hiding this comment.
not apply anymore
|
|
||
| if (actualToVersion != null) { | ||
| if (actualToVersion.longValue() != expectedToVersion) { | ||
| LOG.warning("toVersion in blob didn't match toVersion seen in metadata; actualToVersion=" + actualToVersion + ", expectedToVersion=" + expectedToVersion); |
There was a problem hiding this comment.
nit: the terms blob and metadata dont apply in the context of this class as we want them to, we can rephrase the msg to something like
toVersion encoded in incoming blob header did not match toVersion requested in transition; the one from blob header will be assumed
There was a problem hiding this comment.
not apply anymore
workeatsleep
force-pushed
the
consumer-read-metadata-blob-version
branch
from
5e4cb58
to
3df6e33
Compare
workeatsleep
force-pushed
the
consumer-read-metadata-blob-version
branch
2 times, most recently
from
e7b81cb
to
d4493aa
Compare
| private final long actualVersion; | ||
|
|
||
| public VersionMismatchException(long expectedVersion, long actualVersion) { | ||
| super("toVersion in blob didn't match toVersion seen in metadata; actualToVersion=" + actualVersion + ", expectedToVersion=" + expectedVersion); |
There was a problem hiding this comment.
nit: metadata doesn't really mean apply in this context, we can drop it from the error msg
There was a problem hiding this comment.
will change to "did not match toVersion requested in transition"
| @@ -261,6 +267,59 @@ private static void addActor(HollowWriteStateEngine stateEngine, int id) { | |||
| stateEngine.add("Actor", rec); | |||
| } | |||
|
|
|||
| @Test | |||
| public void testReadBlobVersionInitialInconsistent() throws IOException { | |||
| testReadBlobVersion(2, 3, 1, true); | |||
There was a problem hiding this comment.
nit: (as discussed) can we add some asserts corresponding to versions that the consumer transitioned to
| HollowBlobHeader header = headerReader.readHeader(in); | ||
| if (expectedVersion != 0 && !isInitialUpdate) { |
There was a problem hiding this comment.
(as discussed), maybe better approach to parse headers in HollowDataHolder before applying delta, so that we dont have to pass isInitialUpdate and expectedVersion context all the way down here
workeatsleep
force-pushed
the
consumer-read-metadata-blob-version
branch
from
d4493aa
to
4810852
Compare
workeatsleep
force-pushed
the
consumer-read-metadata-blob-version
branch
from
4810852
to
f3a6b1e
Compare
When the consumer apply a delta transition, it will check the blob version in blob metadata. If this is a transition happen during the app start up, keep the existing behavior, otherwise, throw an exception when there is a mismatch of blob metadata to version and delta to version.