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

[JBPM-10184] Build templates optimization #1600

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -245,26 +245,41 @@ public void onServerInstanceDisconnected(@Observes ServerInstanceDisconnected se
public void onServerInstanceConnected(@Observes ServerInstanceConnected serverInstanceConnected) {
ServerInstance serverInstance = serverInstanceConnected.getServerInstance();

serverTemplatesClients.computeIfPresent(serverInstance.getServerTemplateId(),
(serverTemplateId, clients) -> {
clients.forEach((key, client) -> {
// update regular clients
updateOrBuildClient(client,
serverInstance);

logger.debug("KieServerClient load balancer updated for server template {}",
serverTemplateId.equals(SERVER_TEMPLATE_KEY) ? serverInstance.getServerTemplateId() : serverTemplateId);
});
return clients;
});
ServerInstanceKey serverInstanceKey = serverInstancesById.computeIfAbsent(serverInstance.getServerInstanceId(),s -> {
logger.debug("Server {} connected!", serverInstance);

if (Boolean.parseBoolean(System.getProperty("org.kie.server.startup.detectOpenshiftLoadBalancer", "false"))) {
serverInstancesById.put(serverInstance.getServerInstanceId(),
serverInstance);
}
Comment on lines +251 to +254
Copy link
Contributor

@fjtirado fjtirado Feb 22, 2024

Choose a reason for hiding this comment

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

Suggested change
if (Boolean.parseBoolean(System.getProperty("org.kie.server.startup.detectOpenshiftLoadBalancer", "false"))) {
serverInstancesById.put(serverInstance.getServerInstanceId(),
serverInstance);
}
if (Boolean.getBoolean("org.kie.server.startup.detectOpenshiftLoadBalancer")) {
return serverInstance;
}


serverInstancesById.put(serverInstance.getServerInstanceId(),
serverInstance);
serverTemplatesClients.computeIfPresent(serverInstance.getServerTemplateId(),
(serverTemplateId, clients) -> {
clients.forEach((key, client) -> {
// update regular clients
updateOrBuildClient(client,
serverInstance);

logger.debug("KieServerClient load balancer updated for server template {}",
serverTemplateId.equals(SERVER_TEMPLATE_KEY) ? serverInstance.getServerTemplateId() : serverTemplateId);
});
return clients;
});

serverInstancesById.put(serverInstance.getServerInstanceId(),
Copy link
Contributor

@fjtirado fjtirado Feb 22, 2024

Choose a reason for hiding this comment

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

This line should be removed. serverInstance will be added to the map when this function returns

serverInstance);

KieServicesClient adminClient = adminClients.get(serverInstance.getServerTemplateId());
// update admin clients
updateOrBuildClient(adminClient,
serverInstance);
return serverInstance;
});
if (serverInstance != serverInstanceKey) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Lets do that differently, rather than tracing "isAlreadyRegistered", add a trace when it is registered. If this new registering trace is not present, it can be assumed it has not been registered

// If Server is already connected do not proceed with Artifacts Build
logger.debug("Server {} already registered. Skipping build of containers", serverInstance);
}

KieServicesClient adminClient = adminClients.get(serverInstance.getServerTemplateId());
// update admin clients
updateOrBuildClient(adminClient,
serverInstance);
// once all steps are completed successfully notify other parts interested so the serverClient can actually be used
serverInstanceRegisteredEvent.fire(new ServerInstanceRegistered(serverInstanceConnected.getServerInstance()));
}
Expand Down
Loading