[FLINK-29919] Support operator leader election #434
Conversation
|
cc @morhidi |
gaborgsomogyi
left a comment
gaborgsomogyi
left a comment
There was a problem hiding this comment.
Basically looks good, only questions/nits.
| int exceptionFieldLengthThreshold; | ||
| int exceptionThrowableCountThreshold; | ||
| String labelSelector; | ||
| LeaderElectionConfiguration leaderElectionConfiguration; |
There was a problem hiding this comment.
Nit: any reason why these are not private?
| try { | ||
| new FlinkOperator(operatorConfig); | ||
| } catch (IllegalConfigurationException ice) { | ||
| assertTrue(ice.getMessage().startsWith("Lease name must be defined")); |
There was a problem hiding this comment.
Nit: Maybe we can mention either the feature (leader election) or config name. Such case one don't need to dig for conf name just copy-paste.
There was a problem hiding this comment.
Oh, now see that the feature later mentioned. Maybe adding the key would provide more convenience.
There was a problem hiding this comment.
Good idea, I will add this
|
+1 LGTM overall. Some e2e would be nice. Is this going to be the recommended prod setup or we're not sure yet? |
I think the recommended setup is without leader election as it's a very optional feature. I think most prod settings do not require this and it only adds complication |
3 participants
What is the purpose of the change
Support enabling and configuring leader election for the kubernetes operator to make it possible to run standby operator instances.
Brief change log
Verifying this change
FlinkOperatorTestto cover the leader election config.Does this pull request potentially affect one of the following parts:
CustomResourceDescriptors: noDocumentation