Skip to content

Conversation

@No-SilverBullet
Copy link
Owner

What this PR does:

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:


// initRemoting init remoting
func initRemoting(cfg *Config) {
getty.InitRpcClient(&cfg.GettyConfig, &remoteConfig.SeataConfig{
SeataConfig := remoteConfig.SeataConfig{
Copy link
Owner Author

Choose a reason for hiding this comment

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

should be lowercase , change to seataConfig := xxx

Copy link
Collaborator

Choose a reason for hiding this comment

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

done

mergeMsgMap: &sync.Map{},
}
})
func NewGettyRemoting() *GettyRemoting {
Copy link
Owner Author

Choose a reason for hiding this comment

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

NewGettyRemoting -> newGettyRemoting 包外不能访问

Copy link
Collaborator

@solisamicus solisamicus Dec 20, 2024

Choose a reason for hiding this comment

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

已将 NewGettyRemoting 改为 newGettyRemoting,限制包外访问

Comment on lines 55 to 57
func initSessionManager(gettyConfig *config.Config, seataConfig *config.SeataConfig) {
sessionManager = &SessionManager{
allSessions: sync.Map{},
Copy link
Owner Author

Choose a reason for hiding this comment

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

用sync.Once保证只初始化一次会不会好一点

Copy link
Collaborator

Choose a reason for hiding this comment

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

done


func (f *clientOnResponseProcessor) Process(ctx context.Context, rpcMessage message.RpcMessage) error {
log.Infof("the rm client received clientOnResponse msg %#v from tc server.", rpcMessage)
client := getty.GetGettyRemotingClient()
Copy link
Owner Author

Choose a reason for hiding this comment

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

client 换个名字,不是很清晰的一个命名,maybe gettyClient or something else

Copy link
Collaborator

@solisamicus solisamicus Dec 20, 2024

Choose a reason for hiding this comment

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

改进了变量命名,将 client 改为更明确的 gettyRemotingClient

Comment on lines 42 to 43
msgFutures *sync.Map
mergeMsgMap *sync.Map
Copy link
Owner Author

Choose a reason for hiding this comment

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

我记得这两个好像没用,讨论的时候说是不是可以删除掉

Copy link
Collaborator

Choose a reason for hiding this comment

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

移除了 gettyClientHandler 中未使用的 msgFutures 和 mergeMsgMap 字段

@No-SilverBullet No-SilverBullet merged commit dd541f8 into master Dec 20, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants