Skip to content

Conversation

@mukul1987
Copy link
Contributor

The last applied term index wasn't updated as the method is called by multiple executors and the method was not synchronized.

@mukul1987 mukul1987 self-assigned this Jul 10, 2019
@bshashikant
Copy link
Contributor

Looks good. Pending test results.

}

private void updateLastApplied() {
private synchronized void updateLastApplied() {
Copy link
Contributor

Choose a reason for hiding this comment

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

@mukul1987 Thanks for working on this! The patch looks good to me.
I think we can avoid using synchronized by using a concurrent sorted map. Each applyTransaction removes its corresponding entry from the map and we update lastApplied based on the first entry in the map?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

HDDS-1792 will fix this by using ConcurrentHashSet.

@lokeshj1703
Copy link
Contributor

@mukul1987 The changes look good to me. +1.

@mukul1987 mukul1987 merged commit 0976f6f into apache:trunk Jul 14, 2019
asfgit pushed a commit that referenced this pull request Jul 15, 2019
…rmIndex. Contributed by Mukul Kumar Singh. (#1072)

(cherry picked from commit 0976f6f)
shanthoosh pushed a commit to shanthoosh/hadoop that referenced this pull request Oct 15, 2019
	SAMZA-2228:added LICENCE and NOTICE to samza-tools
amahussein pushed a commit to amahussein/hadoop that referenced this pull request Oct 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants