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

feature: add db realization for distribute lock #3487

Merged
merged 44 commits into from
Aug 16, 2021

Conversation

caohdgege
Copy link
Contributor

Ⅰ. Describe what this PR did

添加db模式下的分布式锁实现

Ⅱ. Does this pull request fix one issue?

fixes #3428

Ⅲ. Why don't you add test cases (unit test/integration test)?

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@caohdgege caohdgege changed the title [WIP]feature: add db realization for distribute lock feature: add db realization for distribute lock Feb 9, 2021
@caohdgege caohdgege changed the title feature: add db realization for distribute lock [WIP]feature: add db realization for distribute lock Feb 25, 2021
@caohdgege caohdgege changed the title [WIP]feature: add db realization for distribute lock feature: add db realization for distribute lock Feb 25, 2021
@caohdgege caohdgege closed this Feb 25, 2021
@caohdgege caohdgege reopened this Feb 25, 2021
# Conflicts:
#	core/src/main/java/io/seata/core/constants/ConfigurationKeys.java
#	core/src/main/java/io/seata/core/store/DistributedLockDO.java
#	script/server/config/file.conf
#	script/server/config/file.properties
#	script/server/config/file.yml
#	server/src/main/java/io/seata/server/Server.java
#	server/src/main/resources/file.conf
@codecov-commenter
Copy link

codecov-commenter commented Jul 7, 2021

Codecov Report

Merging #3487 (663e08d) into develop (4628861) will decrease coverage by 0.18%.
The diff coverage is 0.59%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #3487      +/-   ##
=============================================
- Coverage      40.56%   40.38%   -0.19%     
- Complexity      3070     3071       +1     
=============================================
  Files            682      685       +3     
  Lines          23082    23194     +112     
  Branches        2854     2869      +15     
=============================================
+ Hits            9364     9367       +3     
- Misses         12848    12958     +110     
+ Partials         870      869       -1     
Impacted Files Coverage Δ
...rc/main/java/io/seata/common/util/StringUtils.java 33.18% <ø> (-0.74%) ⬇️
.../main/java/io/seata/config/ConfigurationCache.java 64.35% <0.00%> (-7.08%) ⬇️
...ava/io/seata/core/constants/ConfigurationKeys.java 0.00% <ø> (ø)
...b/sql/distributed/lock/BaseDistributedLockSql.java 0.00% <0.00%> (ø)
...ql/distributed/lock/DistributedLockSqlFactory.java 0.00% <0.00%> (ø)
server/src/main/java/io/seata/server/Server.java 0.00% <0.00%> (ø)
...o/seata/server/coordinator/DefaultCoordinator.java 0.00% <0.00%> (ø)
...in/java/io/seata/server/session/SessionHolder.java 0.00% <0.00%> (ø)
...ver/storage/db/lock/DataBaseDistributedLocker.java 0.00% <0.00%> (ø)
...ata/rm/datasource/undo/AbstractUndoLogManager.java 51.42% <100.00%> (ø)
... and 5 more

Copy link
Contributor

@funky-eyes funky-eyes left a comment

Choose a reason for hiding this comment

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

LGTM

@funky-eyes funky-eyes requested a review from jsbxyyx July 30, 2021 16:23
@lightClouds917
Copy link
Contributor

lightClouds917 commented Aug 15, 2021

I think '分布式锁' use 'Distributed lock' is better than 'Distribute lock'.
Distribute lock 的意思是分配锁,感觉有偏差,而且db这一套实现内部名称也没有统一。

lock_value VARCHAR2(20) NOT NULL,
expire DECIMAL(18) NOT NULL,
PRIMARY KEY (lock_key)
Copy link
Contributor

Choose a reason for hiding this comment

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

是否需要创建时间这个字段?如果出现什么问题,能知道这个记录什么时候insert进去的。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这个我觉得应该不需要。

  1. expire + 配置的distributedLockExpireTime是可以反推出来插入时间的
  2. 不同公司的对于插入时间的字段名一般也不太一样,如果他们有需要可以随时自行加一个default current_timestamp的插入时间

connection.commit();
return true;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

释放锁的做法不是delete,而是update,是为了可重入吗?
那分布式锁表岂不是数据越来越多?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

delete其实更符合分布式锁的正常实现方式,但是在db的时候,我这边测试的话,数据量少的时候,MySQL插入是拿了表锁,因此就很容易造成死锁;然后考虑到我们目前的使用方式中,传入的key都是一样的,也就无需清理,因此就采用了update的形式

Copy link
Contributor

Choose a reason for hiding this comment

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

哦,是的,我们这个场景目前确实是这样的。

Copy link
Contributor

@lightClouds917 lightClouds917 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@jsbxyyx jsbxyyx left a comment

Choose a reason for hiding this comment

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

LGTM

@jsbxyyx jsbxyyx merged commit e2e5fa6 into apache:develop Aug 16, 2021
@wangliang181230 wangliang181230 added this to the 1.5.0 milestone Aug 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

distributed locking needs to be added for the server DB mode
6 participants