Skip to content

Remove RefreshConfig() from backups#4101

Merged
sougou merged 1 commit intovitessio:masterfrom
tinyspeck:revert-refresh-config
Jul 25, 2018
Merged

Remove RefreshConfig() from backups#4101
sougou merged 1 commit intovitessio:masterfrom
tinyspeck:revert-refresh-config

Conversation

@rafael
Copy link
Copy Markdown
Member

@rafael rafael commented Jul 24, 2018

Description

* Currently this is not having the intended behavior. Removing while adding a
  proper fix for this. This effectively reverts:

  vitessio#3063

Signed-off-by: Rafael Chacon <rafael@slack-corp.com>
@rafael rafael requested a review from sougou July 24, 2018 20:55
@rafael rafael changed the title Remove RefreshConfig() Remove RefreshConfig() from backups Jul 24, 2018
@tomkrouper
Copy link
Copy Markdown
Collaborator

Just an FYI, I did test the behavior with setting up the config/mycnf data to match my production /etc/mysql/my.cnf but I ran into an issue where the replacement my.cnf is owned by vitess and set to read-only, only for that user, so mysqld didn't start up properly. I guess what I'm saying is I think this a mysqld vs mysqlctl issue.

Copy link
Copy Markdown
Contributor

@sougou sougou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also eventually remove the function and the grpc support for it. But that can be part of the bigger cleanup. So, this is good for now.

I don't fully understand @tomkrouper's comment. But it seems orthogonal to this fix. So, I'll follow up separately with him.

@sougou sougou merged commit ff2984a into vitessio:master Jul 25, 2018
@rafael rafael deleted the revert-refresh-config branch July 25, 2018 18:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants