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.

Sign up for GitHub

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

Jump to bottom

*: try to support lock/unlock tables function #305

Merged
merged 19 commits into from
Jun 10, 2019
Merged

*: try to support lock/unlock tables function #305

merged 19 commits into from
Jun 10, 2019

Conversation

crazycs520
Copy link
Contributor

@crazycs520 crazycs520 commented Apr 28, 2019 •
edited
Loading

What problem does this PR solve?

  1. Help to support lock/unlock tables function in TiDB.

  2. There are 4 table lock types: READ, READ LOCAL, WRITE, WRITE LOCAL.
    Attention:

  • The WRITE LOCAL is add in TiDB to support TiFlash usage. Mysql doesn't have WRITE LOCAL.
    The WRITE LOCAL means the session with this lock has write/read permission, and the other session is still has read permission.
  • The READ LOCAL is not supported in TiDB, only support the syntax currently. Because of we think is it useless.

What is changed and how it works?

Check List

Tests

  • Unit test

Code changes

Side effects

Related changes

@crazycs520 crazycs520 force-pushed the table_lock branch 8 times, most recently from 798dc77 to 0f49074 Compare April 30, 2019 05:31
@crazycs520 crazycs520 mentioned this pull request May 5, 2019
Merged
@winkyao
Copy link
Contributor

winkyao commented May 6, 2019

Please explain what is WRITE LOCAL

bb7133
bb7133 reviewed May 10, 2019
View reviewed changes
model/model.go Outdated Show resolved Hide resolved
model/model.go Outdated Show resolved Hide resolved
model/model.go Outdated Show resolved Hide resolved
model/model.go Outdated
}

// DBTableID is composed by schema ID and table ID.
type DBTableID struct {
Copy link
Member

Choose a reason for hiding this comment

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

Looks that this is not used anymore, right?

model/model.go Outdated

const (
TableLockNone TableLockType = iota
// TableLockRead means the session with this lock has read permission, other session can't read/write until the lock is released.
Copy link
Member

Choose a reason for hiding this comment

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

IMHO we need to add more detailed explanations. For example, if the locks are exclusive, or can be shared.

Although the behavior of LOCKS will be explained in our official document, detailed comments are still useful here for the developers.

Copy link
Member

Choose a reason for hiding this comment

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

I've checked the implementations, this comment is wrong(other session can't read/write ...), please fix it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for my mistakes, PTAL again.

@crazycs520 crazycs520 mentioned this pull request May 10, 2019
Merged
@bb7133
Copy link
Member

bb7133 commented May 25, 2019

We should also add ErrTableLocked(from pingcap/tidb#10343) related error definitions at parser side.

crazycs520 and others added 5 commits May 27, 2019 10:00
@crazycs520 @bb7133
Update model/model.go
a9d7243 
Co-Authored-By: bb7133 <bb7133@gmail.com>
@crazycs520 @bb7133
Update model/model.go
9140378 
Co-Authored-By: bb7133 <bb7133@gmail.com>
@crazycs520 @bb7133
Update model/model.go
a244916 
Co-Authored-By: bb7133 <bb7133@gmail.com>
@crazycs520
address comment
dbb5e38
@crazycs520
Merge branch 'master' of https://github.com/pingcap/parser into table…
0a76ab7 
…_lock
@codecov
Copy link

codecov bot commented May 27, 2019 •
edited
Loading

Codecov Report

Merging #305 into master will decrease coverage by 0.15%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #305      +/-   ##
==========================================
- Coverage   53.09%   52.94%   -0.16%     
==========================================
  Files          31       31              
  Lines        6575     6575              
==========================================
- Hits         3491     3481      -10     
- Misses       2742     2755      +13     
+ Partials      342      339       -3
Impacted Files Coverage Δ
parser.go 70.58% <ø> (ø) ⬆️
model/ddl.go 76.99% <ø> (ø) ⬆️
model/model.go 34.78% <0%> (-3.47%) ⬇️
ast/ddl.go 53.09% <0%> (-1.35%) ⬇️
lexer.go 97.8% <0%> (-0.05%) ⬇️
ast/misc.go 18.3% <0%> (ø) ⬆️
misc.go 96.42% <0%> (ø) ⬆️
types/field_type.go 31.72% <0%> (ø) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 82be48f...9b7171e. Read the comment docs.

bb7133
bb7133 reviewed May 31, 2019
View reviewed changes
model/model.go Outdated Show resolved Hide resolved
bb7133
bb7133 reviewed May 31, 2019
View reviewed changes
Copy link
Member

@bb7133 bb7133 left a comment

Choose a reason for hiding this comment

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

LGTM

@bb7133
Copy link
Member

bb7133 commented May 31, 2019

PTAL @winkyao @zimulala

@bb7133 bb7133 added the status/LGT1 LGT1 label Jun 5, 2019
winkyao
winkyao previously approved these changes Jun 5, 2019
View reviewed changes
Copy link
Contributor

@winkyao winkyao left a comment

Choose a reason for hiding this comment

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

LGTM

@bb7133 bb7133 added status/LGT2 LGT2 type/enhancement New feature or request and removed status/LGT1 LGT1 labels Jun 10, 2019
@bb7133 bb7133 merged commit 0ea1bf0 into pingcap:master Jun 10, 2019
lzmhhh123 pushed a commit to lzmhhh123/parser that referenced this pull request Jan 17, 2020
@crazycs520 @lzmhhh123
*: try to support lock/unlock tables function (pingcap#305)
80e3ee9
lzmhhh123 added a commit that referenced this pull request Jan 17, 2020
@lzmhhh123 @crazycs520
*: suport alter table set flash replica syntax (#564) (#716)
923ffb7 
* *: try to support lock/unlock tables function (#305)

* *: add admin cleanup table lock syntax (#323)

* *: support alter table set flash replica syntax (#564)

Co-authored-by: crazycs <crazycs520@gmail.com>
lzmhhh123 pushed a commit to lzmhhh123/parser that referenced this pull request Jan 17, 2020
@crazycs520 @lzmhhh123
*: try to support lock/unlock tables function (pingcap#305)
2bd497e
crazycs520 added a commit to crazycs520/parser that referenced this pull request Apr 23, 2020
@crazycs520
*: try to support lock/unlock tables function (pingcap#305)
56ac4c2
This was referenced Apr 23, 2020
Merged
Merged
kennytm pushed a commit that referenced this pull request Apr 24, 2020
@crazycs520
*: try to support lock/unlock tables function (#305) (#827)
b7f2b6e
tiancaiamao pushed a commit to tiancaiamao/parser that referenced this pull request Apr 27, 2021
@crazycs520 @bb7133
*: try to support lock/unlock tables function (pingcap#305)
fa38b94
lyonzhi pushed a commit to lyonzhi/parser that referenced this pull request Apr 25, 2024
@crazycs520 @bb7133
*: try to support lock/unlock tables function (pingcap#305)
d628578
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bb7133 bb7133 bb7133 approved these changes

@winkyao winkyao winkyao left review comments

Assignees
No one assigned
Labels
status/LGT2 LGT2 type/enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@crazycs520 @winkyao @bb7133