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: support RedisLocker#acquireLock use LUA mode #3472

Merged
merged 39 commits into from
May 11, 2021

Conversation

litianyu1992
Copy link
Contributor

Ⅰ. Describe what this PR did

Use Lua to ensure atomicity

Ⅱ. Does this pull request fix one issue?

no

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

jedis mock not support jedis.eval

Ⅳ. Describe how to verify it

acquireLock

Ⅴ. Special notes for reviews

@codecov-io
Copy link

codecov-io commented Jan 19, 2021

Codecov Report

Merging #3472 (2471a47) into develop (6400385) will decrease coverage by 0.32%.
The diff coverage is 31.57%.

❗ Current head 2471a47 differs from pull request most recent head 11afe85. Consider uploading reports for the commit 11afe85 to get more accurate results
Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #3472      +/-   ##
=============================================
- Coverage      52.09%   51.76%   -0.33%     
+ Complexity      3498     3484      -14     
=============================================
  Files            637      638       +1     
  Lines          21105    21154      +49     
  Branches        2618     2619       +1     
=============================================
- Hits           10994    10951      -43     
- Misses          9025     9117      +92     
  Partials        1086     1086              
Impacted Files Coverage Δ Complexity Δ
...n/src/main/java/io/seata/common/io/FileLoader.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
...c/main/java/io/seata/core/lock/AbstractLocker.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...o/seata/server/storage/redis/lock/RedisLocker.java 44.84% <40.38%> (-18.22%) 19.00 <7.00> (+1.00) ⬇️
...n/java/io/seata/common/util/StringFormatUtils.java 0.00% <0.00%> (-92.31%) 0.00% <0.00%> (-13.00%)
...n/src/main/java/io/seata/common/util/IdWorker.java 77.08% <0.00%> (-6.25%) 11.00% <0.00%> (-1.00%)
...erver/storage/file/session/FileSessionManager.java 48.90% <0.00%> (-2.92%) 25.00% <0.00%> (-1.00%)
...rage/redis/store/RedisTransactionStoreManager.java 63.92% <0.00%> (-0.46%) 37.00% <0.00%> (-1.00%)
...tasource/sql/struct/cache/MysqlTableMetaCache.java 83.52% <0.00%> (+1.17%) 11.00% <0.00%> (+1.00%)
.../rm/datasource/exec/mysql/MySQLInsertExecutor.java 72.88% <0.00%> (+3.46%) 14.00% <0.00%> (-2.00%) ⬆️

sb.append(" end ");
sb.append("for i =1, keySize do ");
sb.append(" if(array[i] == 'no') then ");
sb.append("redis.call('HSET',KEYS[i],'").append(XID).append("',ARGV[(i-1)*7+4]);");
Copy link
Contributor

Choose a reason for hiding this comment

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

please optimize the string as a constant

@litianyu1992 litianyu1992 changed the title optimize: RedisLocker#acquireLock use LUA feature: RedisLocker#acquireLock use LUA Jan 19, 2021
} catch (IOException e) {
return;
}
ACQUIRE_LOCK_SHA = jedis.scriptLoad(acquireLockLuaByFile.toString());
Copy link
Contributor

Choose a reason for hiding this comment

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

try (Jedis jedis = JedisPooledFactory.getJedisInstance()) { 放到这比较合适,避免压根文件不存在时还创建一个连接

if (ACQUIRE_LOCK_SHA == null) {
try (Jedis jedis = JedisPooledFactory.getJedisInstance()) {
File luaFile = FileLoader.load(REDIS_LUA_FILE_NAME);
if (luaFile != null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

建议增加日志明确目前竞争锁走的是lua还是pipeline

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

@litianyu1992 litianyu1992 changed the title feature: RedisLocker#acquireLock use LUA feature: add RedisLocker#acquireLock LUA mode Feb 22, 2021
args.add(needLockXid);
for (LockDO lockDO : needLockDOs) {
keys.add(buildLockKey(lockDO.getRowKey()));
args.add(lockDO.getXid());
Copy link
Member

Choose a reason for hiding this comment

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

Is it repeated ?
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已经删除重复参数

Copy link
Member

@slievrly slievrly left a comment

Choose a reason for hiding this comment

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

LGTM

@slievrly slievrly added this to the 1.5.0 milestone Feb 23, 2021
@slievrly slievrly added the module/server server module label Feb 23, 2021
@slievrly slievrly changed the title feature: add RedisLocker#acquireLock LUA mode feature: support RedisLocker#acquireLock use LUA mode Mar 5, 2021
@slievrly slievrly modified the milestones: 1.4.2, 1.5.0 Mar 29, 2021
@codecov-commenter
Copy link

Codecov Report

Merging #3472 (a872192) into develop (fcd8dac) will increase coverage by 0.07%.
The diff coverage is 31.57%.

❗ Current head a872192 differs from pull request most recent head adb98a3. Consider uploading reports for the commit adb98a3 to get more accurate results
Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #3472      +/-   ##
=============================================
+ Coverage      51.89%   51.97%   +0.07%     
+ Complexity      3537     3506      -31     
=============================================
  Files            639      639              
  Lines          21414    21183     -231     
  Branches        2664     2627      -37     
=============================================
- Hits           11113    11010     -103     
+ Misses          9193     9082     -111     
+ Partials        1108     1091      -17     
Impacted Files Coverage Δ Complexity Δ
...n/src/main/java/io/seata/common/io/FileLoader.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
...c/main/java/io/seata/core/lock/AbstractLocker.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...o/seata/server/storage/redis/lock/RedisLocker.java 44.84% <40.38%> (-18.22%) 19.00 <7.00> (+1.00) ⬇️
...g/annotation/datasource/DataSourceProxyHolder.java 0.00% <0.00%> (-75.00%) 0.00% <0.00%> (-3.00%)
...ion/datasource/SeataAutoDataSourceProxyAdvice.java 0.00% <0.00%> (-70.84%) 0.00% <0.00%> (-3.00%)
...on/datasource/SeataAutoDataSourceProxyCreator.java 0.00% <0.00%> (-69.45%) 0.00% <0.00%> (-8.00%)
...io/seata/core/rpc/netty/AbstractNettyRemoting.java 13.42% <0.00%> (-1.35%) 5.00% <0.00%> (ø%)
...o/seata/rm/datasource/AbstractConnectionProxy.java 12.08% <0.00%> (-1.10%) 5.00% <0.00%> (-1.00%)
...n/java/io/seata/rm/datasource/ConnectionProxy.java 24.64% <0.00%> (-1.05%) 13.00% <0.00%> (-1.00%)
...java/io/seata/rm/datasource/ConnectionContext.java 81.44% <0.00%> (-1.04%) 36.00% <0.00%> (-1.00%)
... and 59 more

@slievrly slievrly merged commit 659dd2c into apache:develop May 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module/server server module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants