MINOR: improved exception/warn logging for stream-stream join store settings#13682
MINOR: improved exception/warn logging for stream-stream join store settings#13682ableegoldman wants to merge 2 commits intoapache:trunkfrom
Conversation
ableegoldman
changed the title
There was a problem hiding this comment.
Can we extract this and cherry-pick to 3.5 before the release goes out?
mjsax
left a comment
mjsax
left a comment
There was a problem hiding this comment.
Thanks for the PR Sophie. LGTM.
…or stream-stream join KV store
ableegoldman
force-pushed
the
MINOR-improve-assertWindowSettings-error-msg
branch
from
f63852b to
15e21f7
Compare
vamossagar12
left a comment
vamossagar12
left a comment
There was a problem hiding this comment.
hey @ableegoldman , thanks for the PR. I see it's already approved by Matthias. I had a small comment (you can ignore if it doesn't make sense)
| throw new StreamsException(String.format( | ||
| "The StoreSupplier for join store %s must set supplier.windowSize = joinWindows.windowSize," | ||
| + " found supplier.windowSize = %d, joinWindows.size = %d", | ||
| supplier.name(), supplier.windowSize(), joinWindows.size())); |
There was a problem hiding this comment.
@ableegoldman , I am thinking if the exception message can be rephrased to something like =>
Incorrect value {} used for config {} for StoreSupplier for join store. The correct value should be {}. Do you think that sounds slightly more concise?
|
This PR is being marked as stale since it has not had any activity in 90 days. If you would like to keep this PR alive, please ask a committer for review. If the PR has merge conflicts, please update it with the latest from trunk (or appropriate release branch) If this PR is no longer valid or desired, please feel free to close it. If no activity occurrs in the next 30 days, it will be automatically closed. |
|
This PR has been closed since it has not had any activity in 120 days. If you feel like this |
3 participants
Trying to configure the underlying state stores in a join is pretty awkward, and getting all the parameters right can be particularly frustrating.
At a minimum, we should break up the verification into each individual check so that it's clear which step has failed. It would also help to actually include in the error message how to fix the store parameters to match what is expected, since this is not necessarily obvious -- for example, that retention period has to be exactly the sum of windowSize + gracePeriod, or that the store's windowSize is actually twice the value passed in to
JoinWindows#of(or the newerJoinWindows#ofTimeDifferenceXXXAPIs that replaced it)