orchestrator: initial support #130
Conversation
sougou
requested review from
enisoc and
PrismaPhonic
and removed request for
enisoc
There was a problem hiding this comment.
Most of this seems pretty boilerplate. I assume you tested this out?
Also, I had thought that vorchestrator was going to get integrated directly in with vtctld. Did I misunderstand that? This seems to deploy orchestrator as a standalone deployment?
Also, I see that you said enable_semi_sync should be false. What's the reasoning for recommending that it's false? If this is an important recommendation, it should be documented on that flag ideally.
| @@ -102,7 +106,7 @@ type VitessClusterSpec struct { | |||
| Keyspaces []VitessKeyspaceTemplate `json:"keyspaces,omitempty" patchStrategy:"merge" patchMergeKey:"name"` | |||
|
|
|||
| // ExtraVitessFlags can optionally be used to pass flags to all Vitess components. | |||
| // WARNING: Any flags passed here must be flags that can be accepted by vtgate, vtctld and vttablet. | |||
| // WARNING: Any flags passed here must be flags that can be accepted by vtgate, vtctld, orchestrator and vttablet. | |||
There was a problem hiding this comment.
Is this actually true? If so this seems like a pretty big downside to this integration right now. Have we verified which flags orchestrator does not accept that vtgate, vtctld and vttablet does accept?
There was a problem hiding this comment.
I would think that if Orchestrator is independent of Vitess (is this orchestrator support or vorchestrator support?) that these flags should NOT be passed to orchestrator.
There was a problem hiding this comment.
This should generally be true. This version of Orchestrator is a vitess component. It shares (and requires) the same flags the other components do.
There was a problem hiding this comment.
Ahh, I see. So this is vorchestrator? If it's a Vitess component style of orchestrator, then we might want to clear this language up some. To me as a reader, it sounds like I need to go reference the main orchestrator code base to find what flags it accepts.
In other words, this reads as if it is a breaking change, when it's not.
Yes. I only tested manually.
This is only a POC. This will allow people to play with it and give early feedback. In the endgame, there may be no separate Orchestrator component.
This is actually very important. It's going to be so important that the flag value will be either overridden by the operator, or we may go one step further and ignore it in vttablet itself. For this reason, I'm refraining from changing documentation of other components until we finalize how we're going to enforce these. |
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
Signed-off-by: Anthony Yeh <enisoc@planetscale.com>
Signed-off-by: Anthony Yeh <enisoc@planetscale.com>
Signed-off-by: Anthony Yeh <enisoc@planetscale.com>
Signed-off-by: Anthony Yeh <enisoc@planetscale.com>
This is an experimental support for Orchestrator. The deployment principles are similar to that of vtctld. There are some differences:
disable_active_reparents=trueandenable_semi_sync=falseon the tablets. Right now, the user has to set them manually. This part will be enforced after we finalize the API.I have a corresponding PR on the vitess branch that demonstrates how this works. I'll submit that later.
There is a high likelihood that we'll quickly move away from this. One possibility is to go per-shard, in which case this can become a property in the shard object.
There are no tests because this is only for people to play with. We'll need to write tests once we finalize the API.