-
Notifications
You must be signed in to change notification settings - Fork 8.8k
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: the redis lock key and the session key has conflict #3148
bugfix: the redis lock key and the session key has conflict #3148
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #3148 +/- ##
=============================================
- Coverage 50.46% 50.33% -0.13%
Complexity 3110 3110
=============================================
Files 593 593
Lines 19571 19570 -1
Branches 2427 2396 -31
=============================================
- Hits 9876 9851 -25
Misses 8702 8702
- Partials 993 1017 +24 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM for @caohdgege
Ⅰ. Describe what this PR did
the lock has this key,it is a name of hash:
private static final String DEFAULT_REDIS_SEATA_GLOBAL_LOCK_PREFIX = "SEATA_GLOBAL_LOCK";
the session has this key,it is a name of string:
private static final String DEFAULT_REDIS_SEATA_GLOBAL_PREFIX = "SEATA_GLOBAL_";
They have the same prefix :SEATA_GLOBAL_ ,the
RedisTransactionStoreManager# public List<GlobalSession> readSession(GlobalStatus[] statuses)
,use theredis scan
of key SEATA_GLOBAL_* to get(key).It will cause:
redis.clients.jedis.exceptions.JedisDataException: WRONGTYPE Operation against a key holding the wrong kind of value at redis.clients.jedis.Protocol.processError(Protocol.java:132) at redis.clients.jedis.Protocol.process(Protocol.java:166) at redis.clients.jedis.Protocol.read(Protocol.java:220) at redis.clients.jedis.Connection.readProtocolWithCheckingBroken(Connection.java:318) at redis.clients.jedis.Connection.getBinaryBulkReply(Connection.java:255) at redis.clients.jedis.Connection.getBulkReply(Connection.java:245) at redis.clients.jedis.Jedis.get(Jedis.java:181) at io.seata.server.storage.redis.store.RedisTransactionStoreManager.readSession(RedisTransactionStoreManager.java:230) at io.seata.server.storage.redis.store.RedisTransactionStoreManager.readSession(RedisTransactionStoreManager.java:290) at io.seata.server.storage.redis.session.RedisSessionManager.findGlobalSessions(RedisSessionManager.java:188) at io.seata.server.storage.redis.session.RedisSessionManager.allSessions(RedisSessionManager.java:178) at io.seata.server.coordinator.DefaultCoordinator.timeoutCheck(DefaultCoordinator.java:216) at io.seata.server.coordinator.DefaultCoordinator.lambda$init$20(DefaultCoordinator.java:404) at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511) at java.util.concurrent.FutureTask.runAndReset$$$capture(FutureTask.java:308) at java.util.concurrent.FutureTask.runAndReset(FutureTask.java) at
Ⅱ. 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