Skip to content

Commit be97644

Browse files
dmatejdmatej
committed
Stabilizing DetachAttachITest
- Removed 5 second wait from the AttachCommand - Letting all futures finish before processing
1 parent 7b35590 commit be97644

File tree

2 files changed

+88
-89
lines changed

2 files changed

+88
-89
lines changed

‎appserver/tests/admin/tests/src/test/java/org/glassfish/main/admin/test/progress/DetachAttachITest.java‎

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -95,25 +95,26 @@ public void commandWithProgressStatus() throws Exception {
9595

9696
@Test
9797
public void detachOnesAttachMulti() throws Exception {
98-
ExecutorService pool = Executors.newCachedThreadPool(r -> {
99-
Thread result = new Thread(r);
100-
result.setDaemon(true);
101-
return result;
98+
final ExecutorService pool = Executors.newCachedThreadPool(r -> {
99+
Thread thread = new Thread(r);
100+
thread.setDaemon(true);
101+
thread.setPriority(Thread.MAX_PRIORITY);
102+
return thread;
102103
});
103-
final DetachedTerseAsadminResult result = ASADMIN.execDetached("progress-custom", "8x1");
104-
assertThat(result, asadminOK());
105-
assertNotNull(result.getJobId(), "id");
104+
final DetachedTerseAsadminResult jobIdResult = ASADMIN.execDetached("progress-custom", "8x1");
105+
assertThat(jobIdResult, asadminOK());
106+
assertNotNull(jobIdResult.getJobId(), "id");
106107
final int attachCount = 3;
107-
Collection<Callable<AsadminResult>> attaches = new ArrayList<>(attachCount);
108+
final Collection<Callable<AsadminResult>> attaches = new ArrayList<>(attachCount);
108109
for (int i = 0; i < attachCount; i++) {
109-
attaches.add(() -> ASADMIN.exec("attach", result.getJobId()));
110+
attaches.add(() -> ASADMIN.exec("attach", jobIdResult.getJobId()));
110111
}
111-
List<Future<AsadminResult>> results = pool.invokeAll(attaches);
112-
for (Future<AsadminResult> fRes : results) {
113-
AsadminResult res = fRes.get();
114-
assertThat(res, asadminOK());
115-
assertTrue(res.getStdOut().contains("progress-custom"));
116-
List<ProgressMessage> prgs = ProgressMessage.grepProgressMessages(res.getStdOut());
112+
final List<Future<AsadminResult>> futureResults = pool.invokeAll(attaches);
113+
for (Future<AsadminResult> futureResult : futureResults) {
114+
final AsadminResult result = futureResult.get();
115+
assertThat(result, asadminOK());
116+
assertTrue(result.getStdOut().contains("progress-custom"));
117+
final List<ProgressMessage> prgs = ProgressMessage.grepProgressMessages(result.getStdOut());
117118
assertFalse(prgs.isEmpty());
118119
assertThat(prgs.get(0).getValue(), greaterThanOrEqualTo(0));
119120
assertEquals(100, prgs.get(prgs.size() - 1).getValue());

‎nucleus/core/kernel/src/main/java/com/sun/enterprise/v3/admin/commands/AttachCommand.java‎

Lines changed: 72 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
/*
2+
* Copyright (c) 2025 Contributors to the Eclipse Foundation
23
* Copyright (c) 2008, 2018 Oracle and/or its affiliates. All rights reserved.
34
*
45
* This program and the accompanying materials are made available under the
@@ -22,6 +23,8 @@
2223

2324
import jakarta.inject.Inject;
2425

26+
import java.lang.System.Logger;
27+
2528
import org.glassfish.api.ActionReport;
2629
import org.glassfish.api.I18n;
2730
import org.glassfish.api.Param;
@@ -39,6 +42,9 @@
3942
import org.glassfish.security.services.common.SubjectUtil;
4043
import org.jvnet.hk2.annotations.Service;
4144

45+
import static java.lang.System.Logger.Level.DEBUG;
46+
import static java.lang.System.Logger.Level.ERROR;
47+
import static java.lang.System.Logger.Level.WARNING;
4248
import static org.glassfish.api.admin.AdminCommandState.State.COMPLETED;
4349
import static org.glassfish.api.admin.AdminCommandState.State.PREPARED;
4450
import static org.glassfish.api.admin.AdminCommandState.State.REVERTED;
@@ -60,40 +66,32 @@
6066
@AccessRequired(resource="jobs/job/$jobID", action="attach")
6167
public class AttachCommand implements AdminCommand, AdminCommandListener {
6268

63-
6469
public static final String COMMAND_NAME = "attach";
65-
protected final static LocalStringManagerImpl strings = new LocalStringManagerImpl(AttachCommand.class);
66-
67-
protected AdminCommandEventBroker eventBroker;
68-
protected Job attached;
70+
private static final LocalStringManagerImpl strings = new LocalStringManagerImpl(AttachCommand.class);
71+
private static final Logger LOG = System.getLogger(AttachCommand.class.getName());
6972

7073
@Inject
71-
JobManagerService registry;
74+
private JobManagerService registry;
7275

73-
@Param(primary=true, optional=false, multiple=false)
74-
protected String jobID;
76+
@Param(primary = true, optional = false, multiple = false)
77+
private String jobID;
78+
79+
private AdminCommandEventBroker<?> eventBroker;
80+
private Job attached;
7581

7682
@Override
7783
public void execute(AdminCommandContext context) {
7884
eventBroker = context.getEventBroker();
79-
8085
attached = registry.get(jobID);
81-
JobInfo jobInfo = null;
82-
String jobName = null;
83-
86+
final JobInfo jobInfo;
8487
if (attached == null) {
85-
//try for completed jobs
86-
if (registry.getCompletedJobs(registry.getJobsFile()) != null) {
87-
jobInfo = (JobInfo) registry.getCompletedJobForId(jobID);
88-
}
89-
if (jobInfo != null) {
90-
jobName = jobInfo.jobName;
91-
}
92-
88+
LOG.log(DEBUG, "Trying to find completed job id: {0}", jobID);
89+
jobInfo = registry.getCompletedJobForId(jobID);
90+
} else {
91+
jobInfo = null;
9392
}
94-
95-
attach(attached,jobInfo,context,jobName);
96-
93+
final String jobName = jobInfo == null ? null : jobInfo.jobName;
94+
attach(jobInfo, context, jobName);
9795
}
9896

9997
@Override
@@ -114,13 +112,16 @@ public void onAdminCommandEvent(String name, Object event) {
114112

115113
protected void purgeJob(String jobid) {
116114
try {
115+
Thread.sleep(1000L);
117116
registry.purgeJob(jobid);
118117
registry.purgeCompletedJobForId(jobid);
119118
} catch (Exception ex) {
119+
LOG.log(ERROR, "Failed to purge job with id: {1}", jobid);
120120
}
121121
}
122122

123-
public void attach(Job attached, JobInfo jobInfo, AdminCommandContext context,String jobName) {
123+
124+
private void attach(JobInfo jobInfo, AdminCommandContext context, String jobName) {
124125
ActionReport ar = context.getActionReport();
125126
String attachedUser = SubjectUtil.getUsernamesFromSubject(context.getSubject()).get(0);
126127
if ((attached == null && jobInfo == null) || (attached != null && attached.getName().startsWith("_"))
@@ -129,65 +130,62 @@ public void attach(Job attached, JobInfo jobInfo, AdminCommandContext context,St
129130
ar.setMessage(strings.getLocalString("attach.wrong.commandinstance.id", "Job with id {0} does not exist.", jobID));
130131
return;
131132
}
132-
133-
if (attached != null) {
134-
String jobInitiator = attached.getSubjectUsernames().get(0);
135-
if (!attachedUser.equals( jobInitiator)) {
136-
ar.setActionExitCode(ActionReport.ExitCode.FAILURE);
137-
ar.setMessage(strings.getLocalString("user.not.authorized",
138-
"User {0} not authorized to attach to job {1}", attachedUser, jobID));
139-
return;
140-
}
141-
}
142-
if (attached != null) {
143-
//Very sensitive locking part
144-
AdminCommandEventBroker attachedBroker = attached.getEventBroker();
145-
CommandProgress commandProgress = attached.getCommandProgress();
146-
if (commandProgress == null) {
147-
synchronized (attachedBroker) {
148-
onAdminCommandEvent(AdminCommandStateImpl.EVENT_STATE_CHANGED, attached);
149-
attachedBroker.registerListener(".*", this);
150-
}
151-
} else {
152-
synchronized (commandProgress) {
153-
onAdminCommandEvent(AdminCommandStateImpl.EVENT_STATE_CHANGED, attached);
154-
onAdminCommandEvent(CommandProgress.EVENT_PROGRESSSTATUS_STATE, attached.getCommandProgress());
155-
attachedBroker.registerListener(".*", this);
156-
}
157-
}
158-
synchronized (attached) {
159-
while(attached.getState().equals(PREPARED) ||
160-
attached.getState().equals(RUNNING) ||
161-
attached.getState().equals(RUNNING_RETRYABLE)) {
162-
try {
163-
attached.wait(1000*60*5); //5000L just to be sure
164-
} catch (InterruptedException ex) {}
165-
}
166-
if (attached.getState().equals(COMPLETED) || attached.getState().equals(REVERTED)) {
167-
String commandUser = attached.getSubjectUsernames().get(0);
168-
//In most cases if the user who attaches to the command is the same
169-
//as one who started it then purge the job once it is completed
170-
if ((commandUser != null && commandUser.equals(attachedUser)) && attached.isOutboundPayloadEmpty()) {
171-
purgeJob(attached.getId());
172-
173-
}
174-
ar.setActionExitCode(attached.getActionReport().getActionExitCode());
175-
ar.appendMessage(strings.getLocalString("attach.finished", "Command {0} executed with status {1}",attached.getName(),attached.getActionReport().getActionExitCode()));
176-
}
177-
}
178-
} else {
179-
133+
if (attached == null) {
180134
if (jobInfo != null && (jobInfo.state.equals(COMPLETED.toString()) || jobInfo.state.equals(REVERTED.toString()))) {
181-
182135
//In most cases if the user who attaches to the command is the same
183136
//as one who started it then purge the job once it is completed
184137
if (attachedUser!= null && attachedUser.equals( jobInfo.user)) {
138+
LOG.log(WARNING, "");
185139
purgeJob(jobInfo.jobId);
186-
187140
}
188141
ar.setActionExitCode(ActionReport.ExitCode.SUCCESS);
189142
ar.appendMessage(strings.getLocalString("attach.finished", "Command {0} executed{1}",jobName,jobInfo.exitCode));
190143
}
144+
return;
145+
}
146+
String jobInitiator = attached.getSubjectUsernames().get(0);
147+
if (!attachedUser.equals(jobInitiator)) {
148+
ar.setActionExitCode(ActionReport.ExitCode.FAILURE);
149+
ar.setMessage(strings.getLocalString("user.not.authorized",
150+
"User {0} not authorized to attach to job {1}", attachedUser, jobID));
151+
return;
152+
}
153+
//Very sensitive locking part
154+
AdminCommandEventBroker<?> attachedBroker = attached.getEventBroker();
155+
CommandProgress commandProgress = attached.getCommandProgress();
156+
if (commandProgress == null) {
157+
synchronized (attachedBroker) {
158+
onAdminCommandEvent(AdminCommandStateImpl.EVENT_STATE_CHANGED, attached);
159+
attachedBroker.registerListener(".*", this);
160+
}
161+
} else {
162+
synchronized (commandProgress) {
163+
onAdminCommandEvent(AdminCommandStateImpl.EVENT_STATE_CHANGED, attached);
164+
onAdminCommandEvent(CommandProgress.EVENT_PROGRESSSTATUS_STATE, attached.getCommandProgress());
165+
attachedBroker.registerListener(".*", this);
166+
}
167+
}
168+
synchronized (attached) {
169+
while (PREPARED.equals(attached.getState()) || RUNNING.equals(attached.getState())
170+
|| RUNNING_RETRYABLE.equals(attached.getState())) {
171+
try {
172+
LOG.log(WARNING, "Waiting for job {0}!", attached);
173+
attached.wait();
174+
} catch (InterruptedException e) {
175+
LOG.log(WARNING, "Job {0} interrupted!");
176+
}
177+
}
178+
if (COMPLETED.equals(attached.getState()) || REVERTED.equals(attached.getState())) {
179+
String commandUser = attached.getSubjectUsernames().get(0);
180+
//In most cases if the user who attaches to the command is the same
181+
//as one who started it then purge the job once it is completed
182+
if (attachedUser.equals(commandUser) && attached.isOutboundPayloadEmpty()) {
183+
purgeJob(attached.getId());
184+
}
185+
ar.setActionExitCode(attached.getActionReport().getActionExitCode());
186+
ar.appendMessage(strings.getLocalString("attach.finished", "Command {0} executed with status {1}",
187+
attached.getName(), attached.getActionReport().getActionExitCode()));
188+
}
191189
}
192190
}
193191

0 commit comments

Comments
 (0)