[WIP]handler: add `/db-table?table_ids={...}` #54280

lance6716 · 2024-06-27T08:24:53Z

What problem does this PR solve?

Issue Number: close #54281

Problem Summary:

What changed and how does it work?

curl http://{TiDBIP}:10080/db-table?table_ids={tableID,...}

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No need to test
- I checked and no code files have been changed.

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Documentation

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

None

Signed-off-by: lance6716 <lance6716@gmail.com>

tiprow · 2024-06-27T08:25:11Z

Hi @lance6716. Thanks for your PR.

PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

lance6716 · 2024-06-27T08:26:26Z

docs/tidb_http_api.md

wrong diff calculation 😂

can you point where are changed, diff so large

My IDE writes the correct order for the markdown item list

add the new API under "10. Get database information, table information and tidb info schema version by tableID."

Signed-off-by: lance6716 <lance6716@gmail.com>

codecov · 2024-06-27T09:03:45Z

Codecov Report

Attention: Patch coverage is 75.00000% with 9 lines in your changes missing coverage. Please review.

Project coverage is 56.5699%. Comparing base (08147e7) to head (4df803e).
Report is 25 commits behind head on master.

Additional details and impacted files

@@                Coverage Diff                @@
##             master     #54280         +/-   ##
=================================================
- Coverage   72.8476%   56.5699%   -16.2778%     
=================================================
  Files          1542       1669        +127     
  Lines        436136     619352     +183216     
=================================================
+ Hits         317715     350367      +32652     
- Misses        98837     245475     +146638     
- Partials      19584      23510       +3926

Flag	Coverage Δ
integration	`37.0984% <2.9411%> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
dumpling	`52.9656% <ø> (ø)`
parser	`∅ <ø> (∅)`
br	`52.5347% <ø> (+6.6505%)`	⬆️

Signed-off-by: lance6716 <lance6716@gmail.com>

D3Hunter · 2024-06-28T03:01:48Z

docs/tidb_http_api.md

can you point where are changed, diff so large

D3Hunter · 2024-07-08T09:04:03Z

pkg/server/http_status.go

@@ -248,6 +248,7 @@ func (s *Server) startHTTPServer() {
 	router.Handle("/info", tikvhandler.NewServerInfoHandler(tikvHandlerTool)).Name("Info")
 	router.Handle("/info/all", tikvhandler.NewAllServerInfoHandler(tikvHandlerTool)).Name("InfoALL")
 	// HTTP path for get db and table info that is related to the tableID.
+	router.Handle("/db-table", tikvhandler.NewDBTableHandler(tikvHandlerTool)).Name("DB Table")


prefer /tables, but we have used /db-table, ok to reuse the word

D3Hunter

rest lgtm

D3Hunter · 2024-07-09T05:22:25Z

pkg/server/handler/tests/http_handler_test.go

+	require.NoError(t, err)
+	require.NoError(t, resp.Body.Close())


check response code=400

D3Hunter · 2024-07-09T05:27:37Z

pkg/server/handler/tikvhandler/tikv_handler.go

+				data[tid] = tbl
+			}
+		}
+		handler.WriteData(w, data)


maybe consider wrap data inside another common struct for further extend

{ data: <real-data> }

@mornyx Do you think it's better we add another level JSON object?

/schema does not follow this design. Maybe we should keep them consistent?

D3Hunter · 2024-07-09T05:28:36Z

pkg/server/handler/tikvhandler/tikv_handler.go

+			}
+			tbl, err := getDBTableInfo(schema, int64(tid))
+			if err == nil {
+				data[tid] = tbl


do dashboard need to know it's partially result?

Current way to process the result is verified by dashboard dev.

For the dashboard scenario alone this is as designed.

Signed-off-by: lance6716 <lance6716@gmail.com>

ti-chi-bot · 2024-07-09T06:35:06Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: D3Hunter

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:

~~OWNERS~~ [D3Hunter]

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

ti-chi-bot · 2024-07-09T06:35:09Z

[LGTM Timeline notifier]

Timeline:

2024-07-09 06:35:09.086141354 +0000 UTC m=+339406.321375466: ☑️ agreed by D3Hunter.

Signed-off-by: lance6716 <lance6716@gmail.com>

ti-chi-bot · 2024-07-10T07:36:35Z

@lance6716: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
idc-jenkins-ci-tidb/mysql-test	`4df803e`	link	true	`/test mysql-test`
idc-jenkins-ci-tidb/unit-test	`4df803e`	link	true	`/test unit-test`

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

tiancaiamao · 2024-07-12T08:40:17Z

If it's ready for review, you can remove the [WIP] label @lance6716

lance6716 · 2024-07-15T02:36:22Z

Due to problems on keyvis side, I will open another PR to fix the issue. The API in this PR can't be used smoothly.

lance6716 · 2024-07-15T03:33:34Z

replaced by #54608 (still WIP)

handler: add /schema?table_ids={...}

6842a6e

Signed-off-by: lance6716 <lance6716@gmail.com>

lance6716 commented Jun 27, 2024

View reviewed changes

ti-chi-bot bot removed the do-not-merge/needs-linked-issue label Jun 27, 2024

update resp

dfc58c4

Signed-off-by: lance6716 <lance6716@gmail.com>

add UT

c9789d6

Signed-off-by: lance6716 <lance6716@gmail.com>

ti-chi-bot bot removed the do-not-merge/needs-tests-checked label Jun 27, 2024

lance6716 changed the title ~~[WIP]handler: add /schema?table_ids={...}~~ handler: add /schema?table_ids={...} Jun 27, 2024

ti-chi-bot bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 27, 2024

lance6716 changed the title ~~handler: add /schema?table_ids={...}~~ handler: add /schema?table_ids={...} Jun 27, 2024

lance6716 changed the title ~~handler: add /schema?table_ids={...}~~ [WIP] handler: add /schema?table_ids={...} Jun 27, 2024

ti-chi-bot bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 27, 2024

lance6716 changed the title ~~[WIP] handler: add /schema?table_ids={...}~~ handler: add /schema?table_ids={...} Jun 28, 2024

ti-chi-bot bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 28, 2024

lance6716 added 2 commits July 8, 2024 14:10

Merge branch 'master' of github.com:pingcap/tidb into new-api

c18be83

update

55b5123

Signed-off-by: lance6716 <lance6716@gmail.com>

lance6716 changed the title ~~handler: add /schema?table_ids={...}~~ handler: add /db-table?table_ids={...} Jul 8, 2024

D3Hunter reviewed Jul 8, 2024

View reviewed changes

D3Hunter reviewed Jul 9, 2024

View reviewed changes

check StatusCode

1ad0e86

Signed-off-by: lance6716 <lance6716@gmail.com>

D3Hunter approved these changes Jul 9, 2024

View reviewed changes

ti-chi-bot bot added approved needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Jul 9, 2024

mornyx mentioned this pull request Jul 9, 2024

feat: support multi batches for keyviz schema data pingcap/tidb-dashboard#1700

Merged

lance6716 changed the title ~~handler: add /db-table?table_ids={...}~~ [WIP]handler: add /db-table?table_ids={...} Jul 10, 2024

ti-chi-bot bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 10, 2024

return error when table ID not found

4df803e

Signed-off-by: lance6716 <lance6716@gmail.com>

lance6716 closed this Jul 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP]handler: add `/db-table?table_ids={...}` #54280

[WIP]handler: add `/db-table?table_ids={...}` #54280

lance6716 commented Jun 27, 2024 •

edited

Loading

tiprow bot commented Jun 27, 2024

lance6716 Jun 27, 2024

D3Hunter Jun 28, 2024

lance6716 Jul 9, 2024

codecov bot commented Jun 27, 2024 •

edited

Loading

D3Hunter Jun 28, 2024

D3Hunter Jul 8, 2024

D3Hunter left a comment

D3Hunter Jul 9, 2024

D3Hunter Jul 9, 2024

lance6716 Jul 9, 2024

mornyx Jul 9, 2024

D3Hunter Jul 9, 2024

lance6716 Jul 9, 2024

mornyx Jul 9, 2024

ti-chi-bot bot commented Jul 9, 2024

ti-chi-bot bot commented Jul 9, 2024

ti-chi-bot bot commented Jul 10, 2024

tiancaiamao commented Jul 12, 2024

lance6716 commented Jul 15, 2024

lance6716 commented Jul 15, 2024

		require.NoError(t, err)
		require.NoError(t, resp.Body.Close())

[WIP]handler: add /db-table?table_ids={...} #54280

[WIP]handler: add /db-table?table_ids={...} #54280

Conversation

lance6716 commented Jun 27, 2024 • edited Loading

What problem does this PR solve?

What changed and how does it work?

Check List

Release note

tiprow bot commented Jun 27, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jun 27, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

D3Hunter left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ti-chi-bot bot commented Jul 9, 2024

ti-chi-bot bot commented Jul 9, 2024

[LGTM Timeline notifier]

ti-chi-bot bot commented Jul 10, 2024

tiancaiamao commented Jul 12, 2024

lance6716 commented Jul 15, 2024

lance6716 commented Jul 15, 2024

[WIP]handler: add `/db-table?table_ids={...}` #54280

[WIP]handler: add `/db-table?table_ids={...}` #54280

lance6716 commented Jun 27, 2024 •

edited

Loading

codecov bot commented Jun 27, 2024 •

edited

Loading