Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP [vtctld] backup status #8308

Closed
wants to merge 3 commits into from
Closed

Conversation

ajm188
Copy link
Contributor

@ajm188 ajm188 commented Jun 11, 2021

Description

Demo

Note that for this, I made a quick change to builtinbackupengine's GetBackupStatus method to return a random status, via:

diff --git a/go/vt/mysqlctl/builtinbackupengine.go b/go/vt/mysqlctl/builtinbackupengine.go
index 3f88b5eecd..4888eb6165 100644
--- a/go/vt/mysqlctl/builtinbackupengine.go
+++ b/go/vt/mysqlctl/builtinbackupengine.go
@@ -23,6 +23,7 @@
 	"flag"
 	"fmt"
 	"io"
+	"math/rand"
 	"os"
 	"path"
 	"sync"
@@ -463,7 +464,16 @@ func (be *BuiltinBackupEngine) backupFile(ctx context.Context, params BackupPara
 // This is currently not implemented for builtinbackupengine, so we always
 // return UNKNOWN.
 func (be *BuiltinBackupEngine) GetBackupStatus(ctx context.Context, bh backupstorage.BackupHandle, mfestBytes []byte) (mysqlctlpb.BackupInfo_Status, error) {
-	return mysqlctlpb.BackupInfo_UNKNOWN, nil
+	// pick a random value from the map, only works because there are no gaps in the values
+	i := rand.Intn(len(mysqlctlpb.BackupInfo_Status_name))
+	status := int32(i)
+	if _, ok := mysqlctlpb.BackupInfo_Status_name[status]; !ok {
+		status = int32(mysqlctlpb.BackupInfo_UNKNOWN)
+	}
+
+	return mysqlctlpb.BackupInfo_Status(status), nil
+
+	// return mysqlctlpb.BackupInfo_UNKNOWN, nil
 }
 
 // ExecuteRestore restores from a backup. If the restore is successful

Anyway, the results, showing limiting and details (more to show that the backend impl is correct than anything about the CLI usability -- i am going to change the way we marshal json to convert enums from ints to their actual names separately):

❯ vtctlclient -server ":15999" Backup zone1-101
❯ vtctlclient -server ":15999" Backup zone1-102
❯ vtctldclient --server ":15999" GetBackups commerce/0
2021-06-15.003301.zone1-0000000101
2021-06-15.003310.zone1-0000000102
❯ vtctldclient --server ":15999" GetBackups commerce/0 -l1
2021-06-15.003310.zone1-0000000102
❯ vtctldclient --server ":15999" GetBackups commerce/0 --detailed
{
  "backups": [
    {
      "name": "2021-06-15.003301.zone1-0000000101",
      "directory": "commerce/0",
      "keyspace": "commerce",
      "shard": "0",
      "tablet_alias": {
        "cell": "zone1",
        "uid": 101
      },
      "time": {
        "seconds": "1623717181",
        "nanoseconds": 0
      },
      "engine": "builtin",
      "status": 2
    },
    {
      "name": "2021-06-15.003310.zone1-0000000102",
      "directory": "commerce/0",
      "keyspace": "commerce",
      "shard": "0",
      "tablet_alias": {
        "cell": "zone1",
        "uid": 102
      },
      "time": {
        "seconds": "1623717190",
        "nanoseconds": 0
      },
      "engine": "builtin",
      "status": 1
    }
  ]
}

Related Issue(s)

Checklist

  • Tests were added or are not required
  • Documentation was added or is not required

Deployment Notes

@ajm188 ajm188 force-pushed the am_backup_status branch from a2dd86c to 0169037 Compare June 11, 2021 12:24
ajm188 added 3 commits June 14, 2021 20:17
…le/BackupEngine interfaces to support populating BackupInfo.Status

Signed-off-by: Andrew Mason <amason@slack-corp.com>
Signed-off-by: Andrew Mason <amason@slack-corp.com>
Signed-off-by: Andrew Mason <amason@slack-corp.com>
@ajm188 ajm188 force-pushed the am_backup_status branch from da5ad47 to 38e842e Compare June 15, 2021 00:27
@ajm188 ajm188 requested review from doeg and guidoiaquinti June 15, 2021 00:38
// read-only backups (created by ListBackups). Returns a boolean to indicate
// if the file exists, and an error. Variants of "file not found" errors do
// result in an error, but instead result in (false, nil).
CheckFile(ctx context.Context, filename string) (bool, error)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: I've only implemented this for filebackupstorage and s3backupstorage for now.

@@ -119,6 +125,10 @@ var BackupRestoreEngineMap = make(map[string]BackupRestoreEngine)
// This must only be called after flags have been parsed.
func GetBackupEngine() (BackupEngine, error) {
name := *backupEngineImplementation
return getBackupEngine(name)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

very small method extraction to let me reuse the error codes, but specifying a particular engine name rather than "whatever value the flag pointer has"

@github-actions
Copy link
Contributor

This PR is being marked as stale because it has been open for 30 days with no activity. To rectify, you may do any of the following:

  • Push additional commits to the associated branch.
  • Remove the stale label.
  • Add a comment indicating why it is not stale.

If no action is taken within 7 days, this PR will be closed.

@github-actions github-actions bot added the Stale Marks PRs as stale after a period of inactivity, which are then closed after a grace period. label Jul 17, 2022
@github-actions
Copy link
Contributor

This PR was closed because it has been stale for 7 days with no activity.

@github-actions github-actions bot closed this Jul 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Stale Marks PRs as stale after a period of inactivity, which are then closed after a grace period.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant