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: grpc interceptor xid unbinding problem #5583

Merged
merged 3 commits into from Jun 8, 2023

Conversation

StephenFaust
Copy link
Contributor

  • I have registered the PR changes.

Ⅰ. Describe what this PR did

grpc interceptor xid unbinding problem

Ⅱ. Does this pull request fix one issue?

fixes #5568

@funky-eyes funky-eyes changed the title fix:grpc interceptor xid unbinding problem fix: grpc interceptor xid unbinding problem May 19, 2023
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.

pr 登记到changes下的develop.md中

@funky-eyes funky-eyes changed the title fix: grpc interceptor xid unbinding problem bugfix: grpc interceptor xid unbinding problem May 19, 2023
@funky-eyes
Copy link
Contributor

2.x那块要重新提个pr,我才发现你把一些xid的warn日志也给删了,这块要保留,已便排查相关问题

@StephenFaust
Copy link
Contributor Author

2.x那块要重新提个pr,我才发现你把一些xid的warn日志也给删了,这块要保留,已便排查相关问题

之前那个代码日志打印我认为是不需要的,因为,之前clearContext方法是在onComplete方法中执行,所以这里有可能出现当前线程xid和从上游服务传递的xid不一致(issue有说过,因为onComplete方法其实为一个异步方法,所以原来的代码这里判断了两个xid是否相等,如果不相等,则打印日志,并将xid重新绑定回去,这里的重新绑定我认为没有任何意义,因为,如果你这里就算拿到的是其他请求的xid,那也证明其实被你拿到xid的那个请求已经结束了,这个xid都没有任何意义了,他只是遗留在了线程上下文中),我在pr中将clearContext方法前置到onHalfClose方法中,在这个方法中,我先前置清除遗留的xid(不管是否存在遗留的xid),在进行了绑定操作,这样我一定能保证解绑和绑定是在同一个线程中,因此我这里无需打印任何的日志,因为并不会有异常的情况出现

@StephenFaust
Copy link
Contributor Author

StephenFaust commented May 19, 2023

附上之前的代码

@Override
    public void onComplete() {
        cleanContext();
        target.onComplete();
    }

private void cleanContext() {
        if (StringUtils.isNotBlank(xid) && RootContext.inGlobalTransaction()) {
            String unbindXid = RootContext.unbind();
            BranchType previousBranchType = RootContext.getBranchType();
            if (BranchType.TCC == previousBranchType) {
                RootContext.unbindBranchType();
            }
            if (!xid.equalsIgnoreCase(unbindXid)) {
                RootContext.bind(unbindXid);
                LOGGER.warn("bind xid [{}] back to RootContext", unbindXid);
                if (BranchType.TCC == previousBranchType) {
                    RootContext.bindBranchType(previousBranchType);
                    LOGGER.warn("bind branchType [{}] back to RootContext", previousBranchType);
                }
            }
        }
    }

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

@funky-eyes funky-eyes added type: bug Category issues or prs related to bug. module/integration integration module labels Jun 1, 2023
@funky-eyes funky-eyes added this to the 1.7.0 milestone Jun 1, 2023
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 bd18f08 into apache:develop Jun 8, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module/integration integration module type: bug Category issues or prs related to bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants