prateekm commented on a change in pull request #1008: SAMZA-2174: Throw a
record too large exception for oversized records in changelog
URL: https://github.com/apache/samza/pull/1008#discussion_r297890721
##########
File path:
samza-kv/src/main/scala/org/apache/samza/storage/kv/BaseKeyValueStorageEngineFactory.scala
##########
@@ -162,4 +184,28 @@ trait BaseKeyValueStorageEngineFactory[K, V] extends
StorageEngineFactory[K, V]
keyValueStorageEngineMetrics, batchSize, () => clock.nanoTime())
}
+ def createCachedStore[K, V](enableCache: Boolean, storeName: String,
registry: MetricsRegistry,
+ underlyingStore: KeyValueStore[K, V], cacheSize: Int, batchSize: Int):
KeyValueStore[K, V] = {
+ // maybe wrap with caching
+ val maybeCachedStore = if (enableCache) {
+ val cachedStoreMetrics = new CachedStoreMetrics(storeName, registry)
+ new CachedStore(underlyingStore, cacheSize, batchSize,
cachedStoreMetrics)
+ } else {
+ underlyingStore
+ }
+ maybeCachedStore
+ }
+
+ def createLargeMessageSafeStore(underlyingStore: KeyValueStore[Array[Byte],
Array[Byte]], storeName: String,
Review comment:
Yeah, agree that the new code is significantly more complex and we should
make it more readable. But I think we need to go further than this and fix the
store wrapping pattern and make it consistent. This PR introduces a new
`toBeXXX` variable naming convention + var reassignments, which hurt
readability a lot more. See also the boolean parameter on the other extracted
method.
E.g., the call site for this vs the earlier version will be:
```
createLargeMessageSafeStore(underlyingStore, storeName, dropLargeMessage,
maxMessageSize)
new LargeMessageSafeStore(underlyingStore, storeName, dropLargeMessage,
maxMessageSize)
```
Which is basically the same from a readability perspective.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
