optimize: refactor the redis lock string to hash #3016

lightClouds917 · 2020-08-14T01:36:57Z

Ⅰ. Describe what this PR did

redis lock data structor ，use hash to replace string

Ⅱ. Does this pull request fix one issue?

fixes #3008

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

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

codecov-commenter · 2020-08-14T02:06:02Z

Codecov Report

Merging #3016 into develop will increase coverage by 0.03%.
The diff coverage is 65.82%.

@@              Coverage Diff              @@
##             develop    #3016      +/-   ##
=============================================
+ Coverage      50.39%   50.43%   +0.03%     
- Complexity      3092     3100       +8     
=============================================
  Files            599      599              
  Lines          19531    19488      -43     
  Branches        2412     2360      -52     
=============================================
- Hits            9843     9829      -14     
+ Misses          8697     8644      -53     
- Partials         991     1015      +24

Impacted Files	Coverage Δ	Complexity Δ
...ommon/src/main/java/io/seata/common/Constants.java	`50.00% <ø> (ø)`	`1.00 <0.00> (ø)`
...o/seata/server/storage/redis/lock/RedisLocker.java	`63.96% <65.82%> (+24.12%)`	`19.00 <10.00> (+7.00)`
...obuf/convertor/BranchRegisterRequestConvertor.java	`90.47% <0.00%> (-9.53%)`	`3.00% <0.00%> (ø%)`
...otobuf/convertor/BranchReportRequestConvertor.java	`90.90% <0.00%> (-9.10%)`	`3.00% <0.00%> (ø%)`
...obuf/convertor/BranchRollbackRequestConvertor.java	`92.00% <0.00%> (-8.00%)`	`3.00% <0.00%> (ø%)`
...otobuf/convertor/GlobalBeginResponseConvertor.java	`92.59% <0.00%> (-7.41%)`	`3.00% <0.00%> (ø%)`
...otobuf/convertor/GlobalCommitRequestConvertor.java	`93.75% <0.00%> (-6.25%)`	`3.00% <0.00%> (ø%)`
...otobuf/convertor/GlobalStatusRequestConvertor.java	`93.75% <0.00%> (-6.25%)`	`3.00% <0.00%> (ø%)`
...obuf/convertor/GlobalRollbackRequestConvertor.java	`93.75% <0.00%> (-6.25%)`	`3.00% <0.00%> (ø%)`
...protobuf/convertor/RegisterTMRequestConvertor.java	`94.73% <0.00%> (-5.27%)`	`3.00% <0.00%> (ø%)`
... and 29 more

funky-eyes · 2020-08-14T02:51:03Z

list change hash xid branch1 branch1_info branch2 branch2_info.....
string change hash lock_table rawlock1 branch1_info rawlock2 branch2_info

del:
hgetall xid
branch1->branch1_info json.get("rawlock")->rawlock1
hdel lock_table rawlock1

competition lock：
hsetnx lock_table rawlock1 branch1_info

reentrant lock:
hmget lock_table rawlock1
rawlock1->branch1 json.get(xid).eq(xid)

funky-eyes · 2020-08-14T03:01:58Z

server/src/main/java/io/seata/server/storage/redis/lock/RedisLocker.java

+                lockMap.put(TABLE_NAME, value.getTableName());
+                lockMap.put(ROW_KEY, value.getRowKey());
+                lockMap.put(PK, value.getPk());
+                pipeline.hmset(key, lockMap);


cannot play the role of competition lock

lightClouds917 · 2020-08-18T07:29:29Z

The data structure of redis lock like this now :

funky-eyes

please check my message

funky-eyes · 2020-08-18T07:50:33Z

server/src/main/java/io/seata/server/storage/redis/lock/RedisLocker.java

                String key = readyKeys.get(i);
-                if (result != 1) {
-                    status = result;
+                if (partitions.contains(FAILED)) {


partitions.get(i).contains

funky-eyes · 2020-08-18T07:54:19Z

server/src/main/java/io/seata/server/storage/redis/lock/RedisLocker.java

+                if (successSet.size() > 0) {
+                    Pipeline pipeline2 = jedis.pipelined();
+                    successSet.forEach(locKey ->
+                            pipeline2.hdel(locKey, XID, TRANSACTION_ID, BRANCH_ID, RESOURCE_ID,


del successSet. toArray (new String[0])

funky-eyes · 2020-08-18T08:02:33Z

server/src/main/java/io/seata/server/storage/redis/lock/RedisLocker.java

+
+            Pipeline pipeline1 = jedis.pipelined();
+            needLockKeys.stream().forEachOrdered(
+                    needLockKey -> pipeline1.hmget(needLockKey, XID, TABLE_NAME, PK, BRANCH_ID));


only need to xid

funky-eyes · 2020-08-18T08:13:32Z

server/src/main/java/io/seata/server/storage/redis/lock/RedisLocker.java

                readyKeys.add(key);
            });
            List<Object> results = pipeline.syncAndReturnAll();
-            for (int i = 0; i < results.size(); i++) {
-                Long result = (long)results.get(i);
+            List<Integer> results2 = (List<Integer>) (List) results;


List< Integer > results = (List) (List) pipeline.syncAndReturnAll();

funky-eyes · 2020-08-18T08:17:15Z

server/src/main/java/io/seata/server/storage/redis/lock/RedisLocker.java

+            needReleaseKeys.stream()
+                    .forEach(key -> pipelined.hmget(key, XID, TABLE_NAME, PK, BRANCH_ID));
+            List<Object> existedObjs = pipelined.syncAndReturnAll();
+            List<List<String>> existedLockInfos = (List<List<String>>) (List) existedObjs;


the lock can only be actively unlocked by the owner without having to retrieve it again

funky-eyes · 2020-08-18T08:24:24Z

server/src/main/java/io/seata/server/storage/redis/lock/RedisLocker.java

-            pipeline.sync();
+            Pipeline pipelined = jedis.pipelined();
+            needReleaseKeys.stream()
+                    .forEach(key -> pipelined.hmget(key, XID, TABLE_NAME, PK, BRANCH_ID));


only need to xid

funky-eyes · 2020-08-18T08:26:19Z

server/src/main/java/io/seata/server/storage/redis/lock/RedisLocker.java

+
+            Pipeline pipelined1 = jedis.pipelined();
+            needReleaseKeys.stream().forEach(key ->
+                    pipelined1.hdel(key, XID, TRANSACTION_ID, BRANCH_ID, RESOURCE_ID,


del needReleaseKeys. toArray (new String[0])

funky-eyes · 2020-08-18T08:35:51Z

server/src/main/java/io/seata/server/storage/redis/lock/RedisLocker.java

+                            String[] keys = rowKeyStr.split(SEMICOLON);
+                            Arrays.asList(keys).stream().forEach(rowKey -> {
+                                if (StringUtils.isNotEmpty(rowKey)) {
+                                    pipelined.hdel(rowKey, XID, TRANSACTION_ID, BRANCH_ID,


funky-eyes · 2020-08-18T08:35:58Z

server/src/main/java/io/seata/server/storage/redis/lock/RedisLocker.java

+                                }
+                            });
+                        } else {
+                            pipelined.hdel(rowKeyStr, XID, TRANSACTION_ID, BRANCH_ID, RESOURCE_ID,


funky-eyes · 2020-08-18T08:37:34Z

server/src/main/java/io/seata/server/storage/redis/lock/RedisLocker.java

+            Pipeline pipeline = jedis.pipelined();
+            lockKeys.stream().forEach(key -> pipeline.hget(key, XID));
+            List<Object> existedRowLockXid = pipeline.syncAndReturnAll();
+            List<String> existedXids = (List<String>) (List) existedRowLockXid;


List existedXids = (List) (List) pipeline.syncAndReturnAll();

funky-eyes · 2020-08-18T13:27:08Z

server/src/main/java/io/seata/server/storage/redis/lock/RedisLocker.java

+            String xidLockKey = buildXidLockKey(needLockXid);
+            StringJoiner lockKeysString = new StringJoiner(SEMICOLON);
+            needLockKeys.stream().forEach(lockKey -> lockKeysString.add(lockKey));
+            jedis.hsetnx(xidLockKey, branchId.toString(), lockKeysString.toString());


hset enough

funky-eyes · 2020-08-18T14:31:26Z

server/src/main/java/io/seata/server/storage/redis/lock/RedisLocker.java

+            Pipeline pipeline = jedis.pipelined();
+            lockKeys.stream().forEach(key -> pipeline.hget(key, XID));
+            List<String> existedXids = (List<String>) (List) pipeline.syncAndReturnAll();
+            return existedXids.stream().allMatch(existedXid -> xid.equals(existedXid));


please pay attention to this problem: list All elements are null
existedXids.size>0

funky-eyes · 2020-08-19T01:39:15Z

server/src/main/java/io/seata/server/storage/redis/lock/RedisLocker.java

@@ -60,131 +82,139 @@
     */
    public RedisLocker() {
        logQueryLimit =
-            ConfigurationFactory.getInstance().getInt(ConfigurationKeys.STORE_REDIS_QUERY_LIMIT, DEFAULT_QUERY_LIMIT);
+                ConfigurationFactory.getInstance()


don't need the logQueryLimit

funky-eyes

LGTM

ph3636

Note that the structure is not compatible
LGTM

ph3636 · 2020-08-24T09:37:52Z

common/src/main/java/io/seata/common/Constants.java

-    /** the start time of transaction */
-    public static final String START_TIME  = "start-time";
+    /**
+     * The constant ;


Can be completed or deleted

ph3636 · 2020-08-24T09:38:54Z

server/src/main/java/io/seata/server/storage/redis/lock/RedisLocker.java

@@ -15,23 +15,21 @@
 */
 package io.seata.server.storage.redis.lock;

+import static io.seata.common.Constants.SEMICOLON;
+
+import com.google.common.collect.Lists;


Can be placed below

It seemed that the java package was on the top

It seemed that the java package was on the top

finished

ph3636 · 2020-08-24T09:41:09Z

server/src/main/java/io/seata/server/storage/redis/lock/RedisLocker.java

-                locks =
-                    locks.stream().filter(LambdaUtils.distinctByKey(LockDO::getRowKey)).collect(Collectors.toList());
+            List<LockDO> needLockDOS = convertToLockDO(rowLocks);
+            if (needLockDOS.size() >= 1) {


Remove equal to

The meaning here is the same as before, so be it

The meaning here is the same as before, so be it

finished

ph3636 · 2020-08-24T10:01:45Z

server/src/main/java/io/seata/server/storage/redis/lock/RedisLocker.java

        return DEFAULT_REDIS_SEATA_LOCK_XID_PREFIX + xid;
    }

-    private String getLockKey(String rowKey) {
+    private String buildLockKey(String rowKey) {
        return DEFAULT_REDIS_SEATA_LOCK_PREFIX + rowKey;


Change the names of these two keys for structure error

Change the names of these two keys for structure error

finished

… refactor_redis

l81893521

LGTM for @ph3636

lightClouds917 added 2 commits August 14, 2020 09:06

refactor:the redis lock string to hash

b133981

optimize:code format

0a0fe6e

funky-eyes reviewed Aug 14, 2020

View reviewed changes

lightClouds917 changed the title ~~refactor:the redis lock string to hash~~ [WIP]refactor:the redis lock string to hash Aug 15, 2020

funky-eyes added the TC/store store mode label Aug 15, 2020

lightClouds917 added 2 commits August 18, 2020 11:53

refactor:refactor the redis lock data structure

14f8ec0

refactor:refactor the redis lock data structure

92275de

lightClouds917 changed the title ~~[WIP]refactor:the redis lock string to hash~~ refactor:the redis lock string to hash Aug 18, 2020

fix:the hsetnx response

1f4cca9

funky-eyes requested changes Aug 18, 2020

View reviewed changes

lightClouds917 added 2 commits August 18, 2020 19:16

optimize:the code

f4a0ebb

Merge branch 'develop' of github.com:seata/seata into refactor_redis

93bb511

funky-eyes reviewed Aug 18, 2020

View reviewed changes

lightClouds917 added 2 commits August 19, 2020 09:04

fix:list All elements are null

0197475

format:format the code

d83284f

funky-eyes reviewed Aug 19, 2020

View reviewed changes

funky-eyes added the module/server server module label Aug 19, 2020

lightClouds917 added 2 commits August 19, 2020 09:59

del:the logQueryLimit

3897037

Merge branch 'develop' of github.com:seata/seata into refactor_redis

91360c7

funky-eyes approved these changes Aug 19, 2020

View reviewed changes

funky-eyes requested a review from jsbxyyx August 24, 2020 04:34

ph3636 approved these changes Aug 24, 2020

View reviewed changes

funky-eyes and others added 4 commits August 25, 2020 10:06

Merge branch 'develop' into refactor_redis

024a7e6

Merge branch 'develop' of github.com:seata/seata into refactor_redis

12b8253

optimize:optimize the code

8d3f4cf

optimize:optimize the code

f0ddec3

lightClouds917 added 4 commits August 26, 2020 14:42

Merge branch 'refactor_redis' of github.com:lightClouds917/seata into…

79c9dc4

… refactor_redis

optimize:optimize the code

c226a30

optimize:optimize the code

9978b93

optimize:the code

5da63d9

funky-eyes added this to the 1.4.0 milestone Aug 28, 2020

funky-eyes and others added 2 commits August 28, 2020 17:51

Merge branch 'develop' into refactor_redis

aa93a67

Merge branch 'develop' into refactor_redis

4913506

l81893521 approved these changes Sep 3, 2020

View reviewed changes

funky-eyes changed the title ~~refactor:the redis lock string to hash~~ optimize: refactor the redis lock string to hash Sep 3, 2020

funky-eyes merged commit fc0c9ce into apache:develop Sep 3, 2020

l81893521 pushed a commit to l81893521/seata that referenced this pull request Oct 22, 2020

optimize: refactor the redis lock string to hash (apache#3016)

3b630a0

hicf pushed a commit to hicf/seata that referenced this pull request Nov 15, 2020

optimize: refactor the redis lock string to hash (apache#3016)

22d26a1

optimize: refactor the redis lock string to hash #3016

optimize: refactor the redis lock string to hash #3016

Conversation

lightClouds917 commented Aug 14, 2020 • edited by funky-eyes

Ⅰ. Describe what this PR did

Ⅱ. Does this pull request fix one issue?

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

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

codecov-commenter commented Aug 14, 2020 • edited

Codecov Report

funky-eyes commented Aug 14, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lightClouds917 commented Aug 18, 2020

funky-eyes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

funky-eyes Aug 18, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

funky-eyes left a comment

Choose a reason for hiding this comment

ph3636 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

l81893521 left a comment

Choose a reason for hiding this comment

lightClouds917 commented Aug 14, 2020 •

edited by funky-eyes

codecov-commenter commented Aug 14, 2020 •

edited

funky-eyes commented Aug 14, 2020 •

edited

funky-eyes Aug 18, 2020 •

edited