Skip to content
This repository has been archived by the owner on May 10, 2022. It is now read-only.

fix: gc task may encounter deadlock #168

Merged
merged 6 commits into from
Oct 26, 2021

Conversation

cauchy1988
Copy link
Contributor

遇到的问题描述:
1、我们在恶劣的生产机器环境下发现pegasus-java-client可能会发生死锁现象
2、因为我们应用一次取的数据很多,所以我们初始化客户端的时候配置的超时时间很大与sessionResetTimeWindowMs恰好相同
3、于是在生产环境因为各种原因读取发生超时,导致了以下调用逻辑而发生了死锁, 这里为了简化逻辑描述,假设我们刚开始配置了两个异步nio线程, 即 ’asyncWorkers' 设置成2, 这两个线程分别记做 线程-1, 线程-2 ; 然后我们有3个replicaserver,java client初始化的时候 产生的ReplicaSession结构体和replicaserver 1:1 对应, 这里记做 Replicasession1 , ReplicaSession2, ReplicaSession3; 并且每个 ReplicaSession 在连接对应的replicaserver时,内部的nettychannel会绑定一个nio线程, 这里也为了描述方便,设定对应绑定的关系如下: ReplicaSession1---线程1, ReplicaSession2---线程2, ReplicaSession3---线程1; 发生死锁的流程图如下:
image
4、 如上图, 发生死锁的原因是因为, ReplicaSession.addTimer 的超时流程 会被随机调度到 线程1 或者是 线程2; 然后发生超时后 tryNotifyFailureWithSeqID 方法 会调用closeSession函数来关闭session,但是关闭session的同步操作要在对应的Replicasession里的nettychannel绑定好的线程内部执行的; 于是就可能发生图上的交叉情况 发生死锁;
5、 这个死锁的概率 常规情况应该是概率很小的, 和我司的极限使用 情况有关系
6、给出的修复方法 是在初始化ReplicaSession的时候 就将 超时调度的线程 和 nettychannel线程强制设置成1个, 那么就不会发生上图交叉的情况 而发生死锁了

@foreverneverer
Copy link
Contributor

foreverneverer commented Oct 13, 2021

我们也出现过超时线程无法清理timeout请求,导致请求堆积的场景。我们只是简单的猜测是机器资源导致清理线程hang住,但实际一直没有定位到原因,非常感谢你的工作。

如果是死锁引起的,一个简单的方法,是否可以直接把超时调度线程用别的线程池接管?

@foreverneverer
Copy link
Contributor

nettychannel线程强制设置成1个

这个变更应该不够合理,这会导致配置netty线程池个数的参数失效吧

@cauchy1988
Copy link
Contributor Author

nettychannel线程强制设置成1个

这个变更应该不够合理,这会导致配置netty线程池个数的参数失效吧

确实 影响的因该是 : 如果asyncworker数 大于 replicaserver数, 那么就会有闲置的线程
然后 我觉得你说的对, 额外设置专门处理超时任务的线程 也可以解决

@cauchy1988
Copy link
Contributor Author

nettychannel线程强制设置成1个

这个变更应该不够合理,这会导致配置netty线程池个数的参数失效吧
我再改改;

@cauchy1988
Copy link
Contributor Author

我们也出现过超时线程无法清理timeout请求,导致请求堆积的场景。我们只是简单的猜测是机器资源导致清理线程hang住,但实际一直没有定位到原因,非常感谢你的工作。

如果是死锁引起的,一个简单的方法,是否可以直接把超时调度线程用别的线程池接管?

加了专门处理超时任务的线程池, 帮review下

@foreverneverer foreverneverer changed the title fix extreme deadlock fix: gc task may encounter deadlock Oct 13, 2021
@foreverneverer
Copy link
Contributor

我们也出现过超时线程无法清理timeout请求,导致请求堆积的场景。我们只是简单的猜测是机器资源导致清理线程hang住,但实际一直没有定位到原因,非常感谢你的工作。
如果是死锁引起的,一个简单的方法,是否可以直接把超时调度线程用别的线程池接管?

加了专门处理超时任务的线程池, 帮review下

我建议你可以在压测环境下(大量timeout)长时间运行测试一下,以尽量保证变更不会引起其他问题

@foreverneverer
Copy link
Contributor

请使用mvn spotless:apply 格式化你的代码

@cauchy1988
Copy link
Contributor Author

请使用mvn spotless:apply 格式化你的代码

ok

@cauchy1988
Copy link
Contributor Author

spotless:apply

image
感觉没有效果,是执行别的maven命令嘛

@cauchy1988 cauchy1988 force-pushed the fix_extreme_deadlock branch from 6119fbc to 67e52e8 Compare October 13, 2021 10:08
@foreverneverer
Copy link
Contributor

spotless:apply

image 感觉没有效果,是执行别的maven命令嘛

ci环境可能存在问题,你先再多测试测试这个PR吧

@foreverneverer
Copy link
Contributor

[WARN] /home/runner/work/pegasus-java-client/pegasus-java-client/src/main/java/com/xiaomi/infra/pegasus/client/PegasusClientInterface.java:73: Javadoc comment at column 0 has parse error. Details: no viable alternative at input '   *' while parsing JAVADOC_TAG [AtclauseOrder]
Audit done.
Warning:  src/main/java/com/xiaomi/infra/pegasus/client/PegasusClientInterface.java:[73] (javadoc) AtclauseOrder: Javadoc comment at column 0 has parse error. Details: no viable alternative at input '   *' while parsing JAVADOC_TAG

Java Doc格式需要修改 参考:https://github.com/XiaoMi/pegasus-java-client/pull/170/files#diff-4306d39d8e3a0d08bf4387c0d6941a057f93908c4348aaa241f9f06291f90d7aR72

@cauchy1988
Copy link
Contributor Author

[WARN] /home/runner/work/pegasus-java-client/pegasus-java-client/src/main/java/com/xiaomi/infra/pegasus/client/PegasusClientInterface.java:73: Javadoc comment at column 0 has parse error. Details: no viable alternative at input '   *' while parsing JAVADOC_TAG [AtclauseOrder]
Audit done.
Warning:  src/main/java/com/xiaomi/infra/pegasus/client/PegasusClientInterface.java:[73] (javadoc) AtclauseOrder: Javadoc comment at column 0 has parse error. Details: no viable alternative at input '   *' while parsing JAVADOC_TAG

Java Doc格式需要修改 参考:https://github.com/XiaoMi/pegasus-java-client/pull/170/files#diff-4306d39d8e3a0d08bf4387c0d6941a057f93908c4348aaa241f9f06291f90d7aR72

done

foreverneverer
foreverneverer previously approved these changes Oct 26, 2021
@@ -70,7 +70,7 @@ public PegasusTableInterface openTable(String tableName, int backupRequestDelayM
* @param tableOptions control the table feature, such as open backup-request, compress and etc,
* see {@link TableOptions}
* @return
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 Author

Choose a reason for hiding this comment

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

done

@foreverneverer foreverneverer self-requested a review October 26, 2021 02:11
Copy link
Contributor

@levy5307 levy5307 left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution! @cauchy1988

@levy5307 levy5307 merged commit e2cc860 into XiaoMi:master Oct 26, 2021
@Smityz Smityz mentioned this pull request Dec 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants