Repository navigation
SAMZA-2174: Throw a record too large exception for oversized records in changelog - #1008
Merged
cameronlee314 merged 12 commits intoJul 31, 2019
Merged
Conversation
PawasChhokra
changed the title
jmakes
reviewed
Apr 25, 2019
jmakes
reviewed
Apr 25, 2019
jmakes
reviewed
Apr 25, 2019
rmatharu-zz
reviewed
May 6, 2019
shanthoosh
reviewed
May 9, 2019
…okra/samza into changelog_large_message # Conflicts: # samza-core/src/main/scala/org/apache/samza/config/StorageConfig.scala # samza-kv/src/main/scala/org/apache/samza/storage/kv/BaseKeyValueStorageEngineFactory.scala
cameronlee314
approved these changes
Jun 26, 2019
cameronlee314
left a comment
cameronlee314
left a comment
Contributor
There was a problem hiding this comment.
LGTM
@prateekm could you please take one more look?
prateekm
requested changes
Jun 26, 2019
prateekm
reviewed
Jun 26, 2019
prateekm
reviewed
Jun 27, 2019
cameronlee314
approved these changes
Jul 30, 2019
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Right now, serialization of values happens after we create a CachedStore.
CachedStore does a put/putAll during flush, after it has stored the large message in its cache. This means that the large message will be stored in the cache but rejected when it reaches the addition to the changelog in the LoggedStore.
One option is to let the message remain in the cache and reject the message when it reaches the LoggedStore.
The other option is to serialize the message first in order to decipher the size of the message, then check the size of this message, and later invoke the CachedStore if required.
We are going to go ahead with both these options that the user can enable with a config value.
2 config values:
First option: expect.large.message
Second option: drop.large.messages
Behaviour (True:T, False:F):
TF, TT: throw RecordTooLargeException before adding to cached store
FT: ignore the large message (cached store will store it but it won’t be written to the db)
FF: current state as it is right now, kv store ingests the value but code breaks while adding to changelog stream
@prateekm Kindly take a look.