Skip to content
This repository has been archived by the owner on Dec 8, 2021. It is now read-only.

block user accessing importing tables by RENAME TABLE #544

Open
lance6716 opened this issue Jan 4, 2021 · 5 comments
Open

block user accessing importing tables by RENAME TABLE #544

lance6716 opened this issue Jan 4, 2021 · 5 comments
Labels
feature-request This issue is a feature request

Comments

@lance6716
Copy link
Contributor

Feature Request

Is your feature request related to a problem? Please describe:

When lightning is doing its job, user still can access importing tables, so he may see non-consistent data or interfere checksum processing.

Describe the feature you'd like:

using RENAME TABLE to hide importing tables, both in cluster initiation and online importing scenarios. In order to handle lightning not gracefullly exit, we might save recoving renaming SQLs to a table so user could copy-paste it 😂

Describe alternatives you've considered:

  • downstream already has a technique to detect killed lightning like "ImportMode", so maybe we could let downstream recover renaming instead of user.
  • use LOCK TABLES rather than RENAME, but require TiDB enable-lock-tables in config

Teachability, Documentation, Adoption, Optimization:

@lance6716 lance6716 added the feature-request This issue is a feature request label Jan 4, 2021
@glorv
Copy link
Contributor

glorv commented Jan 13, 2021

I have looked into this issue a bit, there are pros and cons for both of the two solutions:

  1. Rename the table name during importing
  • pros
    • Available for all TiDB versions and easy to implement
  • cons
    • Can't avoid reading/writing to the renamed table
    • Use may create a new table with the original table which will cause rename back failure.
    • How to recover if lightning exit before finished
    • Other use may be confused about the renamed table.
  1. Use TiDB lock table feature
  • pros
    • easy to use. only need to lock the table before starting restoreTable within a new session, and close the session after finished.
    • User friendly. User can get a table locked error if they try to visit the table
    • Have no all the cons of RENAME approach
  • cons
    • Not a GA feature. Must enable by config. In most case, the target cluster won't enable this feature.

@glorv
Copy link
Contributor

glorv commented Jan 13, 2021

@kennytm
Copy link
Collaborator

kennytm commented Jan 13, 2021

actually table locking is not sufficient to protect against Lightning failing 🤔. When Lightning crash, the session holding the lock will be broken and the other sessions will be able to access the inconsistent table.

@glorv
Copy link
Contributor

glorv commented Jan 13, 2021

actually table locking is not sufficient to protect against Lightning failing 🤔. When Lightning crashes, the session holding the lock will be broken and the other sessions will be able to access the inconsistent table.

Yes, but I think this is a feature rather than an issue? If lightning exited abnormally, we should allow the user to do something either by manually do some query to check the table status or just delete the table as a cleanup.

@IANTHEREAL
Copy link
Collaborator

Consider another scenario where data is imported in batches. If concurrent import is required between batches, how to support it?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request This issue is a feature request
Projects
None yet
Development

No branches or pull requests

4 participants