Skip to content

Conversation

@Berkof
Copy link
Contributor

@Berkof Berkof commented Aug 10, 2021

Thank you for submitting the pull request to the Apache Ignite.

In order to streamline the review of the contribution
we ask you to ensure the following steps have been taken:

The Contribution Checklist

  • There is a single JIRA ticket related to the pull request.
  • The web-link to the pull request is attached to the JIRA ticket.
  • The JIRA ticket has the Patch Available state.
  • The pull request body describes changes that have been made.
    The description explains WHAT and WHY was made instead of HOW.
  • The pull request title is treated as the final commit message.
    The following pattern must be used: IGNITE-XXXX Change summary where XXXX - number of JIRA issue.
  • A reviewer has been mentioned through the JIRA comments
    (see the Maintainers list)
  • The pull request has been checked by the Teamcity Bot and
    the green visa attached to the JIRA ticket (see TC.Bot: Check PR)

Notes

If you need any help, please email dev@ignite.apache.org or ask anу advice on http://asf.slack.com #ignite channel.

Comment on lines 284 to 288
public void cancelAllTasks() {
gatheringInProgress.values().forEach(ctx -> ctx.futureGather().cancel(true));

gatheringInProgress.clear();
}
Copy link
Member

Choose a reason for hiding this comment

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

This can be is called from StatisticsManager.disableOperations() as a reaction on configuration changes.
And StatisticsManager.processObsolescence() that is called from scheduler can put into gatheringInProgress in parallel.
Seems, both StatisticsManager methods are marked synchronized, but actually don't check the state under lock.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It won't totally fix the possibility of failure cause we can change state during processObsolescence execution. But I move state check directly into it to improve readability and lower this chance a bit.

Comment on lines +209 to +211
if (!active) {
newCtx.cancel();
gatheringInProgress.remove(key);
Copy link
Member

Choose a reason for hiding this comment

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

Do you read 'active' flag and remove key under busylock?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not here, it's just a fast way to skip scheduling if active=false.

Berkof and others added 2 commits September 8, 2021 19:18
Co-authored-by: Andrew V. Mashenkov <AMashenkov@users.noreply.github.com>
@Berkof Berkof closed this Oct 8, 2021
@Berkof
Copy link
Contributor Author

Berkof commented Oct 8, 2021

Replaced with #9423

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.

2 participants