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

bugfix: fix jedis unlock fail #2897

Merged
merged 10 commits into from
Jul 28, 2020
Merged

bugfix: fix jedis unlock fail #2897

merged 10 commits into from
Jul 28, 2020

Conversation

funky-eyes
Copy link
Contributor

@funky-eyes funky-eyes commented Jul 16, 2020

Ⅰ. Describe what this PR did

fix jedis unlock fail

Ⅱ. Does this pull request fix one issue?

fix #2898

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

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@codecov-commenter
Copy link

codecov-commenter commented Jul 16, 2020

Codecov Report

Merging #2897 into develop will decrease coverage by 0.00%.
The diff coverage is 0.00%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #2897      +/-   ##
=============================================
- Coverage      50.27%   50.27%   -0.01%     
- Complexity      3047     3049       +2     
=============================================
  Files            599      599              
  Lines          19410    19415       +5     
  Branches        2381     2381              
=============================================
+ Hits            9758     9760       +2     
- Misses          8674     8679       +5     
+ Partials         978      976       -2     
Impacted Files Coverage Δ Complexity Δ
...o/seata/server/storage/redis/lock/RedisLocker.java 36.28% <0.00%> (-1.68%) 10.00 <0.00> (ø)
...in/java/io/seata/server/session/GlobalSession.java 84.16% <0.00%> (+0.45%) 72.00% <0.00%> (+1.00%)
...o/seata/server/coordinator/DefaultCoordinator.java 55.15% <0.00%> (+0.51%) 29.00% <0.00%> (+1.00%)

@funky-eyes funky-eyes requested a review from jsbxyyx July 16, 2020 09:04
@funky-eyes funky-eyes added this to the 1.4.0 milestone Jul 16, 2020
@@ -144,6 +144,7 @@ public boolean releaseLock(String xid, List<Long> branchIds) {
if (CollectionUtils.isNotEmpty(keys)) {
Pipeline pipeline = null;
Iterator<String> it = keys.iterator();
List<String> delKeys = new ArrayList<>();
while (it.hasNext()) {
String key = it.next();
LockDO lock = JSON.parseObject(jedis.get(key), LockDO.class);
Copy link
Contributor

Choose a reason for hiding this comment

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

It can be changed to mget

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

@@ -152,13 +153,15 @@ public boolean releaseLock(String xid, List<Long> branchIds) {
if (pipeline == null) {
pipeline = jedis.pipelined();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

It can be deleted and the following with delKeys judgment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

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.

What is the meaning of logQueryLimit?

relieve redis query pressure? If so, why not delete in batches? Will memory be OOM?

@funky-eyes
Copy link
Contributor Author

What is the meaning of logQueryLimit?

relieve redis query pressure? If so, why not delete in batches? Will memory be OOM?

  1. The maximum value of a single query is used to relieve the pressure of redis reading a large amount of data at one time
  2. Use the pipeline to make a one-time request to delete

Copy link
Contributor

@ph3636 ph3636 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 added the TC/store store mode label Jul 27, 2020
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

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 merged commit 8a63c4f into apache:develop Jul 28, 2020
l81893521 pushed a commit to l81893521/seata that referenced this pull request Oct 22, 2020
hicf pushed a commit to hicf/seata that referenced this pull request Nov 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TC/store store mode
Projects
None yet
Development

Successfully merging this pull request may close these issues.

store.mode redis unlock fail
6 participants