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

fix(tidb): fix script integrationtest.sh #2519

Merged

Conversation

purelind
Copy link
Collaborator

  • Correct start pd cmd params.
  • Specify using store tikv for testing.

Signed-off-by: purelind <purelind@gmail.com>
Signed-off-by: purelind <purelind@gmail.com>
@ti-chi-bot ti-chi-bot bot requested review from jayl1e and wuhuizuo October 16, 2023 09:07
@ti-chi-bot
Copy link

ti-chi-bot bot commented Oct 16, 2023

I have already done a preliminary review for you, and I hope to help you do a better job.
This pull request contains changes to the integrationtest.sh script in the tidb folder. Specifically, it corrects the startup command parameters for the PD server and specifies to use the TiKV store for testing.

The changes look good and do not introduce any obvious issues. However, it is not clear why the TiKV path is set to the PD address. It would be better to set the TiKV path to the address of the TiKV server that is being used for testing.

Suggestion:

  1. Set the TiKV path to the address of the TiKV server that is being used for testing.
  2. Double-check that the changes do not cause any unintended side effects.

@ti-chi-bot ti-chi-bot bot added the size/S label Oct 16, 2023
@purelind
Copy link
Collaborator Author

purelind commented Oct 16, 2023

/hold
wait until all test cases passed

@purelind
Copy link
Collaborator Author

Currently, running with store tikv will result in the following errors.

ERRO[0663] 8 tests failed
ERRO[0663] run test [executor/admin] err: sql:admin checksum table cache_admin_table_with_index_test;: failed to run query
"admin checksum table cache_admin_table_with_index_test;"
 around line 66,
we need(172):
admin checksum table cache_admin_table_with_index_test;
Db_name Table_name      Checksum_crc64_xor      Total_kvs       Total_bytes
executor__admin cache_admin_table_with_index_test       0       2       2

but got(172):
admin checksum table cache_admin_table_with_index_test;
Db_name Table_name      Checksum_crc64_xor      Total_kvs       Total_bytes
executor__admin cache_admin_table_with_index_test       0       0       0

ERRO[0663] run test [executor/executor] err: sql:change pump to node_state ='paused' for node_id 'pump1';: failed to run query
"change pump to node_state ='paused' for node_id 'pump1';"
 around line 693,
we need(132):
change pump to node_state ='paused' for node_id 'pump1';
Error 1105 (HY000): URL scheme must be http, https, unix, or unixs:
change
but got(132):
change pump to node_state ='paused' for node_id 'pump1';
Error 1105 (HY000): node pump, id pump1 from etcd 127.0.0.1:2379 not found

ERRO[0663] run test [expression/builtin] err: sql:select * from tb5 where cast(a as unsigned int)=0;: failed to run query
"select * from tb5 where cast(a as unsigned int)=0;"
 around line 280,
we need(156):
select * from tb5 where cast(a as unsigned int)=0;
a       b
Level   Code    Message
Warning 1690    constant 1.844674407370955e+20 overflows bigint
select * from tb5 whe
but got(156):
select * from tb5 where cast(a as unsigned int)=0;
a       b
Level   Code    Message
Warning 1690    evaluation failed: constant 184467440737095500000 overflows LongLong

ERRO[0663] run test [expression/charset_and_collation] err: sql:admin recover index t a;: run "admin recover index t a;" at line 359 err Error 1105 (HY000): [components/tidb_query_executors/src/table_scan_executor.rs:425]: Data is corrupted, missing data for NOT NULL column (offset = 0)
ERRO[0663] run test [expression/issues] err: sql:select * from t where field('A', a collate utf8mb4_general_ci, b) > 1;: failed to run query
"select * from t where field('A', a collate utf8mb4_general_ci, b) > 1;"
 around line 1445,
we need(79):
select * from t where field('A', a collate utf8mb4_general_ci, b) > 1;
a       b
sele
but got(79):
select * from t where field('A', a collate utf8mb4_general_ci, b) > 1;
a       b
a       A
ERRO[0663] run test [planner/cascades/integration] err: sql:select /*+ HASH_AGG() */ a, count(distinct a) from t;: failed to run query
"select /*+ HASH_AGG() */ a, count(distinct a) from t;"
 around line 135,
we need(78):
select /*+ HASH_AGG() */ a, count(distinct a) from t;
a       count(distinct a)
1       2

but got(78):
select /*+ HASH_AGG() */ a, count(distinct a) from t;
a       count(distinct a)
2       2

ERRO[0663] run test [planner/core/plan_cache] err: sql:select * from IDT_20290 where col2 * 049015787697063065230692384394107598316198958.1850509 >= 659971401668884663953087553591534913868320924.5040396 and col2 = 869042976700631943559871054704914143535627349.9659934;: failed to run query
"select * from IDT_20290 where col2 * 049015787697063065230692384394107598316198958.1850509 >= 659971401668884663953087553591534913868320924.5040396 and col2 = 869042976700631943559871054704914143535627349.9659934;"
 around line 209,
we need(263):
select * from IDT_20290 where col2 * 049015787697063065230692384394107598316198958.1850509 >= 659971401668884663953087553591534913868320924.5040396 and col2 = 869042976700631943559871054704914143535627349.9659934;
Error 1690 (22003): DECIMAL value is out of range
but got(263):
select * from IDT_20290 where col2 * 049015787697063065230692384394107598316198958.1850509 >= 659971401668884663953087553591534913868320924.5040396 and col2 = 869042976700631943559871054704914143535627349.9659934;
Error 1690 (22003):  value is out of range in ''
ERRO[0663] run test [session/vars] err: sql:SELECT variable_value FROM mysql.tidb WHERE variable_name = 'tikv_gc_enable';: failed to run query
"SELECT variable_value FROM mysql.tidb WHERE variable_name = 'tikv_gc_enable';"
 around line 3,
we need(98):
SELECT variable_value FROM mysql.tidb WHERE variable_name = 'tikv_gc_enable';
variable_value
SET G
but got(98):
SELECT variable_value FROM mysql.tidb WHERE variable_name = 'tikv_gc_enable';
variable_value
true

Copy link
Contributor

@Defined2014 Defined2014 left a comment

Choose a reason for hiding this comment

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

pingcap/tidb#47697 fixed all problems for real TiKV, I think this PR could merge now.

Signed-off-by: purelind <purelind@gmail.com>
Signed-off-by: purelind <purelind@gmail.com>
@ti-chi-bot
Copy link

ti-chi-bot bot commented Oct 17, 2023

I have already done a preliminary review for you, and I hope to help you do a better job.

Review of "fix(tidb): fix script integrationtest.sh"

Summary

This pull request makes two changes to the integrationtest.sh script in the TiDB repository:

  1. It corrects the parameters passed to the bin/pd-server command.
  2. It adds a new script, integrationtest_with_tikv.sh, which sets up PD and TiKV instances for use with tests.

Potential Problems

The changes in this pull request appear to be straightforward and limited in scope, so there are no major issues that jump out. However, there are a few minor things to consider:

  • The new integrationtest_with_tikv.sh script is missing a shebang line (#! /usr/bin/env bash). This should be added.
  • It's not entirely clear why the pipelined option is being disabled in the tikv.toml file. It may be worth adding a comment to explain this decision.

Suggestions

  • Add a shebang line to the integrationtest_with_tikv.sh script.
  • Consider adding a comment to explain why the pipelined option is being disabled in the tikv.toml file.

Overall, the changes in this pull request seem reasonable and well-contained, so I would recommend approving it.

Copy link
Contributor

@Defined2014 Defined2014 left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-chi-bot ti-chi-bot bot added the lgtm label Oct 17, 2023
@ti-chi-bot
Copy link

ti-chi-bot bot commented Oct 17, 2023

[LGTM Timeline notifier]

Timeline:

  • 2023-10-17 08:21:46.630943873 +0000 UTC m=+1731704.218054018: ☑️ agreed by Defined2014.
  • 2023-10-17 08:51:53.957025705 +0000 UTC m=+1733511.544135852: ✖️🔁 reset by purelind.
  • 2023-10-17 09:00:32.810569374 +0000 UTC m=+1734030.397679519: ☑️ agreed by Defined2014.

@ti-chi-bot
Copy link

ti-chi-bot bot commented Oct 17, 2023

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by: Defined2014

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

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@purelind
Copy link
Collaborator Author

/unhold

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants