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) support ecosystem tool binaries for TiDB #133

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

bb7133
Copy link
Member

@bb7133 bb7133 commented Oct 9, 2024

As described, I plan to introduce more commands to trigger BR(binary) / Dumpling / Lightning(binary) and TiCDC, so that we're able to write tests with those tools.

@bb7133 bb7133 changed the title support ecosystem tool binaries for TiDB (WIP) support ecosystem tool binaries for TiDB Oct 9, 2024
tmpDir := fmt.Sprintf("/tmp/%s_%s", source, uuid)

// Generate the SQL statements
backupSQL := fmt.Sprintf("BACKUP TABLE `%s` TO '%s'", source, tmpDir)
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems that already supports backup and restore SQL, maybe we can use SQL directly instead of use comments.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, when I added 'backup_and_restore' command, I was thinking br binaries. However, it might be OK to just use SQL now and only try br binary when needed.

Copy link
Member Author

Choose a reason for hiding this comment

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

mmm...it is not straightforward to use SQL directly:

BACKUP TABLE T TO 'LOCAL:///TMP/BR';
Destination	Size	BackupTS	Queue Time	Execution Time
local:///TMP/BR	3687	453115446250176572	2024-10-09 11:24:13	2024-10-09 11:24:13
DROP TABLE T;
RESTORE TABLE T FROM 'LOCAL:///TMP/BR';
Destination	Size	BackupTS	Cluster TS	Queue Time	Execution Time
local:///TMP/BR	3687	453115446250176572	453115447167680565	2024-10-09 11:24:17	2024-10-09 11:24:17

The return rows of BACKUP and RESTORE are time-specific.

Copy link
Contributor

Choose a reason for hiding this comment

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

we could use --disable_result_log to disable print return rows. I think decoupling these functions will make it more convenient to use.

return path, nil
}

func (t *tester) importTableStmt(path, target string) string {
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto, use SQL directly is better.

@@ -202,7 +237,7 @@ func isTiDB(db *sql.DB) bool {
return true
}

func (t *tester) addConnection(connName, hostName, userName, password, db string) {
func (t *tester) addConnection(connName, hostName, port, userName, password, db string) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems port is useless here.

Copy link
Member Author

Choose a reason for hiding this comment

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

port is needed when adding connection "downstream" for the TiCDC downstream cluster.

Copy link
Contributor

@Defined2014 Defined2014 Oct 10, 2024

Choose a reason for hiding this comment

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

I got it, it's designed for the future!

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.

2 participants