Skip to content

Add multiple locks which can be obtained based on segment name in lookupOrCreateFSM#4159

Merged
npawar merged 2 commits intoapache:masterfrom
npawar:loggin_fixes
Apr 24, 2019
Merged

Add multiple locks which can be obtained based on segment name in lookupOrCreateFSM#4159
npawar merged 2 commits intoapache:masterfrom
npawar:loggin_fixes

Conversation

@npawar
Copy link
Contributor

@npawar npawar commented Apr 24, 2019

Adding segment name based locks instead of synchronizing the method lookupOrCreateFSM. The latter prevents even other segments from entering the method and accessing their own fsm. A stall in a single call (could be on zk access, gc) can create a ripple effect delay in all segments waiting on that method

Copy link
Member

@kishoreg kishoreg left a comment

Choose a reason for hiding this comment

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

I might be wrong here. Cant parse the indentation properly on phone.

There is possibility that fsm can be null in the end rt? I thought it’s better to lookup the fsmmap in the end.

@npawar
Copy link
Contributor Author

npawar commented Apr 24, 2019

I might be wrong here. Cant parse the indentation properly on phone.

There is possibility that fsm can be null in the end rt? I thought it’s better to lookup the fsmmap in the end.

The idea is to lookup in the fsmMap, and use that if not null. If it is null, then create a new instance of fsm and put it in the map. The fsm cannot be null after the if-else if-else block. Which place are you referring to?

@kishoreg
Copy link
Member

kishoreg commented Apr 24, 2019

My bad, I thought there were two if blocks (outer and inner). In other words, we dont need a lock to check the existence in fsmMap we need the lock if the fsm does not exists.

But it's not a big deal in this code path.

Copy link
Member

@kishoreg kishoreg 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
Contributor

@sunithabeeram sunithabeeram left a comment

Choose a reason for hiding this comment

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

LGTM.

@npawar npawar merged commit 8464997 into apache:master Apr 24, 2019
@npawar npawar deleted the loggin_fixes branch April 24, 2019 22:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants