VTOrc Cleanup - Configs, APIs and old UI#11356
Merged
GuptaManan100 merged 50 commits intovitessio:mainfrom Sep 30, 2022
Merged
Conversation
Contributor
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
If a new flag is being introduced:
If a workflow is added or modified:
Bug fixes
Non-trivial changes
New/Existing features
Backward compatibility
|
3 tasks
GuptaManan100
force-pushed
the
vtorc-config-cleanup
branch
from
5e560ab to
68b2776
Compare
…zation of database Signed-off-by: Manan Gupta <manan@planetscale.com>
… execution on a replica Signed-off-by: Manan Gupta <manan@planetscale.com>
Signed-off-by: Manan Gupta <manan@planetscale.com>
Signed-off-by: Manan Gupta <manan@planetscale.com>
…port Signed-off-by: Manan Gupta <manan@planetscale.com>
Signed-off-by: Manan Gupta <manan@planetscale.com>
Signed-off-by: Manan Gupta <manan@planetscale.com>
Signed-off-by: Manan Gupta <manan@planetscale.com>
Signed-off-by: Manan Gupta <manan@planetscale.com>
Signed-off-by: Manan Gupta <manan@planetscale.com>
…ages Signed-off-by: Manan Gupta <manan@planetscale.com>
Signed-off-by: Manan Gupta <manan@planetscale.com>
…xport port for the old UI Signed-off-by: Manan Gupta <manan@planetscale.com>
Signed-off-by: Manan Gupta <manan@planetscale.com>
Signed-off-by: Manan Gupta <manan@planetscale.com>
Signed-off-by: Manan Gupta <manan@planetscale.com>
Signed-off-by: Manan Gupta <manan@planetscale.com>
Signed-off-by: Manan Gupta <manan@planetscale.com>
Signed-off-by: Manan Gupta <manan@planetscale.com>
Signed-off-by: Manan Gupta <manan@planetscale.com>
…ompatibility Signed-off-by: Manan Gupta <manan@planetscale.com>
Signed-off-by: Manan Gupta <manan@planetscale.com>
Signed-off-by: Manan Gupta <manan@planetscale.com>
GuptaManan100
force-pushed
the
vtorc-config-cleanup
branch
from
b7318a7 to
66a101c
Compare
…endency on vtorc package Signed-off-by: Manan Gupta <manan@planetscale.com>
Signed-off-by: Manan Gupta <manan@planetscale.com>
…o-server Signed-off-by: Manan Gupta <manan@planetscale.com>
Signed-off-by: Manan Gupta <manan@planetscale.com>
GuptaManan100
added
Type: Internal Cleanup
Component: VTOrc
GuptaManan100
requested review from
ajm188,
deepthi,
derekperkins,
dkhenry,
frouioui,
harshit-gangal,
mattlord,
rohit-nayak-ps,
shlomi-noach and
systay
as code owners
GuptaManan100
commented
Sep 30, 2022
Comment on lines
+269
to
+271
| #### Old UI Removal and Replacement | ||
|
|
||
| The old UI that VTOrc inherited from `Orchestrator` has been removed. A replacement UI, more consistent with the other Vitess binaries has been created. |
Contributor
Author
There was a problem hiding this comment.
Release notes documentation is complete
GuptaManan100
commented
Sep 30, 2022
Comment on lines
-54
to
-55
| COPY --from=builder --chown=vitess:vitess /vt/src/vitess.io/vitess/web/orchestrator /vt/web/orchestrator | ||
| COPY --from=builder --chown=vitess:vitess /vt/src/vitess.io/vitess/web/vtorc /vt/web/vtorc |
Contributor
Author
There was a problem hiding this comment.
Since /web/vtorc and /web/orchestrator have been removed, we don't need to copy them in the lite images.
GuptaManan100
commented
Sep 30, 2022
Comment on lines
-153
to
-157
| - vtorc web ui: | ||
| - vtorc ui: | ||
| http://localhost:13000 | ||
|
|
||
| - vtorc debug ui: | ||
| http://localhost:13200 |
Contributor
Author
There was a problem hiding this comment.
We no longer have the old UI. Website docs have to be changed for this. Same change has been made for all the other examples as well.
GuptaManan100
commented
Sep 30, 2022
Comment on lines
+667
to
+669
| func Subtract(lhs, rhs string) (string, error) { | ||
| lhsSet, err := parseMysql56GTIDSet(lhs) | ||
| if err != nil { |
Contributor
Author
There was a problem hiding this comment.
This function was needed because we need to find the difference between the executed gtid set of the replica and primary to find the errant gtids. Earlier VTOrc used to run select gtid_subtract on the MySQL connection to the replica, but since we don't connect to the MySQL replica anymore, we have to use an internal function.
Contributor
Author
There was a problem hiding this comment.
There are sufficient tests that are added to this function to verify that it works just as exected.
GuptaManan100
commented
Sep 30, 2022
| return nil | ||
| } | ||
|
|
||
| // TODO: Simplify the callers and delete this function |
Contributor
Author
There was a problem hiding this comment.
Will do this later.
deepthi
approved these changes
Sep 30, 2022
This was referenced Oct 1, 2022
3 tasks
dbussink
added a commit
to dbussink/vitess
that referenced
this pull request
Feb 20, 2023
This was missed in vitessio#11356 where this flag was deprecated and removed from most other places. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>
3 tasks
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
2 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.
Description
Since VTOrc was forked from
Orchestrator, it inherited a lot of configurations that don't make sense for the Vitess use-case.All of such configurations have been removed in this PR.
VTOrc ignores the configurations that it doesn't understand. So old configurations can be kept around on upgrading and won't cause any issues.
They will just be ignored.
For all the configurations that are kept, flags have been added for them and the flags are the desired way to pass these configurations going forward.
The config file will be deprecated and removed in upcoming releases. The following is a list of all the configurations that are kept and the associated flags added.
--sqlite-data-file--instance-poll-time--snapshot-topology-interval--reasonable-replication-lag--audit-file-location--audit-to-backend--audit-to-syslog--audit-purge-duration--recovery-period-block-duration--prevent-cross-cell-failover--lock-shard-timeout--wait-replicas-timeout--topo-information-refresh-duration--recovery-poll-durationThe config file still takes precedence over the flags that can now be specified.
Apart from configurations, some flags from VTOrc have also been removed -
siblingdestinationdiscoveryskip-unresolveskip-unresolve-checknoopbinlogstatementgrab-electionpromotion-ruleskip-continuous-registrationenable-database-updateignore-raft-setuptagThe CLI code which wasn't being used has also been cleaned up. The old API and UI have also been removed in this PR. Along with that the
web/vtorcdirectory is also removed. We have already added the new UI and ported over the APIs deemed necessary in #11370.This PR also does the release notes documentation for these changes and also the changes in #11370.
Flags changes required for VTOrc are also done as part of this PR.
Related Issue(s)
package go/vt/vtorc/app#11301package go/vt/vtorc/logic#11302Checklist
Deployment Notes