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

IGNITE-20861 P2P prevent to load already deployed class from different node on SHARED mode #11041

Merged
merged 16 commits into from
Jul 28, 2024

Conversation

zstan
Copy link
Contributor

@zstan zstan commented Nov 14, 2023

…t node on SHARED mode

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 [email protected] or ask anу advice on http://asf.slack.com #ignite channel.

@zstan zstan force-pushed the ignite-20861 branch 3 times, most recently from a64b1ed to 62610ed Compare November 14, 2023 17:15
@zstan zstan force-pushed the ignite-20861 branch 2 times, most recently from 3859496 to e14cc7b Compare November 25, 2023 17:47

synchronized (mux) {
// Check obsolete request.
if (isDeadClassLoader(meta))
return null;

Collection<GridDeployment> created = getDeployments();
Copy link
Contributor

Choose a reason for hiding this comment

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

It should be in the If closure bellow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done thanks

if (meta.deploymentMode() == SHARED) {
for (GridDeployment dep0 : created) {
// hot redeploy from same node
if (dep0.participants().containsKey(meta.senderNodeId()) || dep0.undeployed())
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we skipping participan deployment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because the case of this part is to find participant by nodeId and register if such is not found

Copy link
Contributor Author

Choose a reason for hiding this comment

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

check code a bit below, if participant contains - skip, if not - register

IgniteBiTuple<Class<?>, Throwable> cls = dep0.deployedClass(meta.className(), meta.alias());

if (cls.getKey() != null && cls.getValue() == null) {
addParticipant((SharedDeployment)dep0, meta);
Copy link
Contributor

Choose a reason for hiding this comment

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

Here you move all partisipants for a class meta to the deploymnt, but bellow we might update the partisipant list in the class meta.
May we try to update the partisipant list at first or don't do the update because the new list won't use?

if (!skipSearchDeployment)
dep = (SharedDeployment)searchDeploymentCache(meta);
else
dep = null;
Copy link
Contributor

Choose a reason for hiding this comment

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

We can assignt the variable as null at definition.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done thanks

if (cls.getKey() != null && cls.getValue() == null) {
addParticipant((SharedDeployment)dep0, meta);
skipSearchDeployment = true;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why don't bracke the loop when you find the deployment?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, I think we can assigned it into dep variable and brake the root while loop.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because in common - getDeployments() returns the collection an we need to update all participants

@zstan zstan merged commit be24636 into apache:master Jul 28, 2024
6 of 7 checks passed
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