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

Lightning wants CONFIG privilege to query keyspace name and it confuses on-prem users #54232

Closed
kennytm opened this issue Jun 26, 2024 · 5 comments · Fixed by #54243
Closed
Labels
affects-7.5 This bug affects the 7.5.x(LTS) versions. affects-8.1 This bug affects the 8.1.x(LTS) versions. affects-8.2 component/lightning This issue is related to Lightning of TiDB. report/customer Customers have encountered this bug. severity/minor type/enhancement The issue or PR belongs to an enhancement.

Comments

@kennytm
Copy link
Contributor

kennytm commented Jun 26, 2024

Bug Report

Please answer these questions before submitting your issue. Thanks!

1. Minimal reproduce step (Required)

Run Lightning in logical mode with default configuration, especially leaving [tikv-importer] keyspace-name empty.

Do not use the root user to restore to the downstream TiDB. Rather, create a new user and grant SELECT,INSERT,UPDATE,DELETE,CREATE,DROP,ALTER on *.* to that user (as described in the doc).

2. What did you expect to see? (Required)

Everything works normally.

3. What did you see instead (Required)

There is a warning saying the CONFIG privilege is needed.

[2024/06/22 06:41:00.296 +00:00] [WARN] [lightning.go:565] ["unable to get keyspace name, lightning will use empty keyspace name"] [error="Error 1227 (42000): Access denied; you need (at least one of) the CONFIG privilege(s) for this operation"]

4. What is your TiDB version? (Required)

Lightning v7.5.1, v7.5.2, v8.1.0

@kennytm kennytm added type/enhancement The issue or PR belongs to an enhancement. severity/minor component/lightning This issue is related to Lightning of TiDB. report/customer Customers have encountered this bug. affects-8.2 labels Jun 26, 2024
@kennytm
Copy link
Contributor Author

kennytm commented Jun 26, 2024

I think demoting the log level from WARN to INFO/DEBUG, and/or hiding the 1227 error is enough.

@mzhang77
Copy link

There is another related message in log file, it's better to hide it too.

[2024/06/26 13:01:21.193 +00:00] [WARN] [import.go:404] ["check isRaftKV2 failed"] [error="Error 1227 (42000): Access denied; you need (at least one of) the CONFIG privilege(s) for this operation"] [errorVerbose="Error 1227 (42000): Access denied; you need (at least one of) the CONFIG privilege(s) for this operation\ngithub.com/pingcap/errors.AddStack\n\t/root/go/pkg/mod/github.com/pingcap/[email protected]/errors.go:178\ngithub.com/pingcap/errors.Trace\n\t/root/go/pkg/mod/github.com/pingcap/[email protected]/juju_adaptor.go:15\ngithub.com/pingcap/tidb/br/pkg/lightning/common.IsRaftKV2\n\t/workspace/source/tidb/br/pkg/lightning/common/util.go:677\ngithub.com/pingcap/tidb/br/pkg/lightning/importer.NewImportControllerWithPauser\n\t/workspace/source/tidb/br/pkg/lightning/importer/import.go:402\ngithub.com/pingcap/tidb/br/pkg/lightning/importer.NewImportController\n\t/workspace/source/tidb/br/pkg/lightning/importer/import.go:293\ngithub.com/pingcap/tidb/br/pkg/lightning.(*Lightning).run\n\t/workspace/source/tidb/br/pkg/lightning/lightning.go:584\ngithub.com/pingcap/tidb/br/pkg/lightning.(*Lightning).RunOnceWithOptions\n\t/workspace/source/tidb/br/pkg/lightning/lightning.go:389\nmain.main.func2\n\t/workspace/source/tidb/br/cmd/tidb-lightning/main.go:94\nmain.main\n\t/workspace/source/tidb/br/cmd/tidb-lightning/main.go:95\nruntime.main\n\t/usr/local/go/src/runtime/proc.go:267\nruntime.goexit\n\t/usr/local/go/src/runtime/asm_amd64.s:1650"]

@kennytm
Copy link
Contributor Author

kennytm commented Jun 26, 2024

  1. This check is triggered only in local backend. Since local backend requires direct access to TiKV and PD already which is effectively worse than SUPER privilege I think requiring CONFIG additionally is fine?
  2. Unlike keyspace-name, the storage.engine is actually well-documented, except that "raft-kv2" lived under the name "partitioned-raft-kv". So it is reasonable an on-prem user may be using the new engine and Lightning's behavior should respect the choice.
  3. I think we can also fetch the config directly through gRPC, but the cost does not really justify against simply changing the doc to require CONFIG when using local backend + raft-kv2.

@mzhang77
Copy link

Amend document to request CONFIG for physical import is fine, as long as document is clear that it won't confuse lightning user.

@ti-chi-bot ti-chi-bot bot closed this as completed in eec1d4f Jul 4, 2024
@kennytm
Copy link
Contributor Author

kennytm commented Jul 4, 2024

I'll try to implement the TODO mentioned in #54243 some day. If that does not work we'll update the docs to require CONFIG.

@ti-chi-bot ti-chi-bot added affects-7.5 This bug affects the 7.5.x(LTS) versions. affects-8.1 This bug affects the 8.1.x(LTS) versions. labels Jul 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects-7.5 This bug affects the 7.5.x(LTS) versions. affects-8.1 This bug affects the 8.1.x(LTS) versions. affects-8.2 component/lightning This issue is related to Lightning of TiDB. report/customer Customers have encountered this bug. severity/minor type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants