pingcap / dumpling

Dumpling is a fast, easy-to-use tool written by Go for dumping data from the database(MySQL, TiDB...) to local/cloud(S3, GCP...) in multifarious formats(SQL, CSV...).
Apache License 2.0
281 stars 85 forks source link

Add conn session vars #265

Closed recall704 closed 3 years ago

recall704 commented 3 years ago

What problem does this PR solve?

fix resetDBWithSessionParams if value is int or float

What is changed and how it works?

try to parse params values string to int or float.

lichunzhu commented 3 years ago

@recall704 What's the difference between this and params introduced in https://github.com/pingcap/dumpling/pull/111 and https://github.com/pingcap/dumpling/pull/167?

recall704 commented 3 years ago
#!/bin/bash

HOST="127.0.0.1"
PORT=3306
USER="root"
PSW="mypasswd"
DATA_DIR="data2"

rm rf -$DATA_DIR && mkdir -p $DATA_DIR

./bin/dumpling \
    --host=$HOST \
    --port=$PORT \
    --user=$USER \
    --password=$PSW \
    --logfmt=json \
    --loglevel=debug \
    --filter="*.*,!/^(mysql|sys|grafana_ucloud|INFORMATION_SCHEMA|PERFORMANCE_SCHEMA|METRICS_SCHEMA|INSPECTION_SCHEMA)$/.*" \
    --threads=16 \
    --status-addr=:8281 \
    --output=$DATA_DIR \
    --filetype=sql \
    --rows=100000 \
    --params="net_read_timeout=86400,interactive_timeout=28800,wait_timeout=2147483,net_write_timeout=86400" \
    --tidb-mem-quota-query=35433480192

the erorr is

dump failed: Error 1232: Incorrect argument type to variable 'wait_timeout'

you set the session vars with db.ExecContext,

        s := fmt.Sprintf("SET SESSION %s = ?", k)
        _, err := db.ExecContext(tctx, s, v)

and i set the session vars with conn.ExecContext

        query := fmt.Sprintf("SET SESSION %s = %s", k, s)
        _, err := conn.ExecContext(tctx, query)
lichunzhu commented 3 years ago
#!/bin/bash

HOST="127.0.0.1"
PORT=3306
USER="root"
PSW="mypasswd"
DATA_DIR="data2"

rm rf -$DATA_DIR && mkdir -p $DATA_DIR

./bin/dumpling \
    --host=$HOST \
    --port=$PORT \
    --user=$USER \
    --password=$PSW \
    --logfmt=json \
    --loglevel=debug \
    --filter="*.*,!/^(mysql|sys|grafana_ucloud|INFORMATION_SCHEMA|PERFORMANCE_SCHEMA|METRICS_SCHEMA|INSPECTION_SCHEMA)$/.*" \
    --threads=16 \
    --status-addr=:8281 \
    --output=$DATA_DIR \
    --filetype=sql \
    --rows=100000 \
    --params="net_read_timeout=86400,interactive_timeout=28800,wait_timeout=2147483,net_write_timeout=86400" \
    --tidb-mem-quota-query=35433480192

the erorr is

dump failed: Error 1232: Incorrect argument type to variable 'wait_timeout'

you set the session vars with db.ExecContext,

      s := fmt.Sprintf("SET SESSION %s = ?", k)
      _, err := db.ExecContext(tctx, s, v)

and i set the session vars with conn.ExecContext

      query := fmt.Sprintf("SET SESSION %s = %s", k, s)
      _, err := conn.ExecContext(tctx, query)

Yes. dumpling will return an error. I think maybe fixing --params is enough. Could you please explain why do you want to add another argument --session-params?

recall704 commented 3 years ago
#!/bin/bash

HOST="127.0.0.1"
PORT=3306
USER="root"
PSW="mypasswd"
DATA_DIR="data2"

rm rf -$DATA_DIR && mkdir -p $DATA_DIR

./bin/dumpling \
    --host=$HOST \
    --port=$PORT \
    --user=$USER \
    --password=$PSW \
    --logfmt=json \
    --loglevel=debug \
    --filter="*.*,!/^(mysql|sys|grafana_ucloud|INFORMATION_SCHEMA|PERFORMANCE_SCHEMA|METRICS_SCHEMA|INSPECTION_SCHEMA)$/.*" \
    --threads=16 \
    --status-addr=:8281 \
    --output=$DATA_DIR \
    --filetype=sql \
    --rows=100000 \
    --params="net_read_timeout=86400,interactive_timeout=28800,wait_timeout=2147483,net_write_timeout=86400" \
    --tidb-mem-quota-query=35433480192

the erorr is

dump failed: Error 1232: Incorrect argument type to variable 'wait_timeout'

you set the session vars with db.ExecContext,

        s := fmt.Sprintf("SET SESSION %s = ?", k)
        _, err := db.ExecContext(tctx, s, v)

and i set the session vars with conn.ExecContext

        query := fmt.Sprintf("SET SESSION %s = %s", k, s)
        _, err := conn.ExecContext(tctx, query)

Yes. dumpling will return an error. I think maybe fixing --params is enough. Could you please explain why do you want to add another argument --session-params?

OK, I will try to fix in --params

recall704 commented 3 years ago

@lichunzhu It's works now

image

lichunzhu commented 3 years ago

/lgtm

ti-chi-bot commented 3 years ago

[REVIEW NOTIFICATION]

This pull request has been approved by:

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment. After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by writing `/lgtm` in a comment. Reviewer can cancel approval by writing `/lgtm cancel` in a comment.
lichunzhu commented 3 years ago

/merge

ti-chi-bot commented 3 years ago

This pull request has been accepted and is ready to merge.

Commit hash: 304f26c70c089dfe97cf5e72fbf878e4ebe2f078

lichunzhu commented 3 years ago

LGTM. Thanks for your contribution!