Skip to content

OWLS-89106 - Potential fix for pod startup issue in GBU CNE environment after node drain/repave operation #2398

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

Merged
merged 23 commits into from
Jun 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
b33a4a3
Potential fix for pod startup issue in GBU CNE env after node drain/r…
ankedia Jun 8, 2021
9c83cb0
Increase the pod ready wait timeout to 5 min (instead of 2 min) to av…
ankedia Jun 9, 2021
4d6749c
Undo refactoring related changes.
ankedia Jun 9, 2021
291d6d2
Fix to not increment count for introspector job.
ankedia Jun 10, 2021
6d106c7
Potential fix for the race condition in the introspector job completi…
ankedia Jun 10, 2021
3fb128f
Reduce timeout interval to 2 min.
ankedia Jun 10, 2021
70eab49
Add previously removed event validation.
ankedia Jun 11, 2021
4dbbe1c
Changes to check if the cached pod is not found on read then execute …
ankedia Jun 12, 2021
240130d
Fix checkstyle error.
ankedia Jun 12, 2021
b035257
Minor refactoring and increase the timeout value.
ankedia Jun 12, 2021
e988050
Fix comments and minor change.
ankedia Jun 12, 2021
b009898
Minor changes.
ankedia Jun 14, 2021
9a913de
Fix checkstyle error.
ankedia Jun 14, 2021
2093c8f
PR review comments - create new MakeRightDomainOperation instead of u…
ankedia Jun 14, 2021
b3f2362
Revert previous change to create a new make-right operation and imple…
ankedia Jun 14, 2021
b471058
Refactoring changes.
ankedia Jun 14, 2021
7b2bc9f
Check for null call result.
ankedia Jun 15, 2021
5a040cc
More refactoring changes.
ankedia Jun 15, 2021
b974ff8
Added debug message.
ankedia Jun 15, 2021
0616eb6
Minor changes.
ankedia Jun 15, 2021
1c3b6a0
added unit test to test that wait for ready timeout executes make rig…
ankedia Jun 16, 2021
a61ddea
Minor change.
ankedia Jun 16, 2021
23b60ae
Remove unused variable and potential fix for integration test failure.
ankedia Jun 16, 2021
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 @@ -746,7 +746,7 @@ private void addServerToMaps(Map<String, ServerHealth> serverHealthMap,
*/
class MakeRightDomainOperationImpl implements MakeRightDomainOperation {

private final DomainPresenceInfo liveInfo;
private DomainPresenceInfo liveInfo;
private boolean explicitRecheck;
private boolean deleting;
private boolean willInterrupt;
Expand Down Expand Up @@ -851,6 +851,22 @@ public void setInspectionRun() {
inspectionRun = true;
}

@Override
public void setLiveInfo(DomainPresenceInfo info) {
this.liveInfo = info;
}

@Override
public void clear() {
this.liveInfo = null;
this.eventData = null;
this.explicitRecheck = false;
this.deleting = false;
this.willInterrupt = false;
this.inspectionRun = false;
}


@Override
public boolean wasInspectionRun() {
return inspectionRun;
Expand Down
24 changes: 24 additions & 0 deletions operator/src/main/java/oracle/kubernetes/operator/JobWatcher.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import java.util.Optional;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ThreadFactory;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Consumer;
import javax.annotation.Nonnull;
Expand All @@ -25,13 +26,16 @@
import io.kubernetes.client.util.Watchable;
import oracle.kubernetes.operator.TuningParameters.WatchTuning;
import oracle.kubernetes.operator.builders.WatchBuilder;
import oracle.kubernetes.operator.calls.CallResponse;
import oracle.kubernetes.operator.helpers.CallBuilder;
import oracle.kubernetes.operator.helpers.KubernetesUtils;
import oracle.kubernetes.operator.helpers.ResponseStep;
import oracle.kubernetes.operator.logging.LoggingFacade;
import oracle.kubernetes.operator.logging.LoggingFactory;
import oracle.kubernetes.operator.logging.MessageKeys;
import oracle.kubernetes.operator.steps.DefaultResponseStep;
import oracle.kubernetes.operator.watcher.WatchListener;
import oracle.kubernetes.operator.work.NextAction;
import oracle.kubernetes.operator.work.Packet;
import oracle.kubernetes.operator.work.Step;
import oracle.kubernetes.utils.SystemClock;
Expand Down Expand Up @@ -243,6 +247,11 @@ boolean isReady(V1Job job) {
return isComplete(job) || isFailed(job);
}

@Override
boolean onReadNotFoundForCachedResource(V1Job cachedJob, boolean isNotFoundOnRead) {
return false;
}

// Ignore modified callbacks from different jobs (identified by having different creation times) or those
// where the job is not yet ready.
@Override
Expand Down Expand Up @@ -302,6 +311,21 @@ Throwable createTerminationException(V1Job job) {
void logWaiting(String name) {
LOGGER.fine(MessageKeys.WAITING_FOR_JOB_READY, name);
}

@Override
protected DefaultResponseStep<V1Job> resumeIfReady(Callback callback) {
return new DefaultResponseStep<>(null) {
@Override
public NextAction onSuccess(Packet packet, CallResponse<V1Job> callResponse) {
if (isReady(callResponse.getResult()) || callback.didResumeFiber()) {
callback.proceedFromWait(callResponse.getResult());
return doNext(packet);
}
return doDelay(createReadAndIfReadyCheckStep(callback), packet,
getWatchBackstopRecheckDelaySeconds(), TimeUnit.SECONDS);
}
};
}
}

static class DeadlineExceededException extends Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ public interface MakeRightDomainOperation {

void setInspectionRun();

void setLiveInfo(DomainPresenceInfo info);

void clear();

boolean wasInspectionRun();

private static boolean wasInspectionRun(Packet packet) {
Expand Down
85 changes: 85 additions & 0 deletions operator/src/main/java/oracle/kubernetes/operator/PodWatcher.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import java.util.Map;
import java.util.Optional;
import java.util.concurrent.ThreadFactory;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Consumer;
import javax.annotation.Nonnull;
Expand All @@ -28,17 +29,26 @@
import io.kubernetes.client.util.Watchable;
import oracle.kubernetes.operator.TuningParameters.WatchTuning;
import oracle.kubernetes.operator.builders.WatchBuilder;
import oracle.kubernetes.operator.calls.CallResponse;
import oracle.kubernetes.operator.helpers.CallBuilder;
import oracle.kubernetes.operator.helpers.DomainPresenceInfo;
import oracle.kubernetes.operator.helpers.KubernetesUtils;
import oracle.kubernetes.operator.helpers.LegalNames;
import oracle.kubernetes.operator.helpers.PodHelper;
import oracle.kubernetes.operator.helpers.ResponseStep;
import oracle.kubernetes.operator.logging.LoggingFacade;
import oracle.kubernetes.operator.logging.LoggingFactory;
import oracle.kubernetes.operator.logging.MessageKeys;
import oracle.kubernetes.operator.steps.DefaultResponseStep;
import oracle.kubernetes.operator.watcher.WatchListener;
import oracle.kubernetes.operator.work.NextAction;
import oracle.kubernetes.operator.work.Packet;
import oracle.kubernetes.operator.work.Step;

import static oracle.kubernetes.operator.ProcessingConstants.SERVER_NAME;
import static oracle.kubernetes.operator.logging.MessageKeys.EXECUTE_MAKE_RIGHT_DOMAIN;
import static oracle.kubernetes.operator.logging.MessageKeys.LOG_WAITING_COUNT;

/**
* Watches for changes to pods.
*/
Expand Down Expand Up @@ -305,6 +315,8 @@ public Step waitForDelete(V1Pod pod, Step next) {

private abstract static class WaitForPodStatusStep extends WaitForReadyStep<V1Pod> {

public static final int RECHECK_DEBUG_COUNT = 10;

private WaitForPodStatusStep(V1Pod pod, Step next) {
super(pod, next);
}
Expand All @@ -322,6 +334,67 @@ V1ObjectMeta getMetadata(V1Pod pod) {
Step createReadAsyncStep(String name, String namespace, String domainUid, ResponseStep<V1Pod> responseStep) {
return new CallBuilder().readPodAsync(name, namespace, domainUid, responseStep);
}

protected DefaultResponseStep<V1Pod> resumeIfReady(Callback callback) {
return new DefaultResponseStep<>(getNext()) {
@Override
public NextAction onSuccess(Packet packet, CallResponse<V1Pod> callResponse) {

DomainPresenceInfo info = packet.getSpi(DomainPresenceInfo.class);
String serverName = (String)packet.get(SERVER_NAME);
String resource = initialResource == null ? resourceName : getMetadata(initialResource).getName();
if ((info != null) && (callResponse != null)) {
Optional.ofNullable(callResponse.getResult()).ifPresent(result ->
info.setServerPodFromEvent(getPodLabel(result), result));
if (onReadNotFoundForCachedResource(getServerPod(info, serverName), isNotFoundOnRead(callResponse))) {
LOGGER.fine(EXECUTE_MAKE_RIGHT_DOMAIN, serverName, callback.getRecheckCount());
removeCallback(resource, callback);
return doNext(NEXT_STEP_FACTORY.createMakeDomainRightStep(callback, info, getNext()), packet);
}
}

if (isReady(callResponse.getResult()) || callback.didResumeFiber()) {
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to reset the recheckCount here?

Copy link
Member Author

Choose a reason for hiding this comment

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

I moved the recheckCount variable from DomainPresenceInfo to the Callback object since we create a separate callback instance for each resource. The WaitForReady step registers a new Callback instance in the resumeWhenReady method and the previous callback will be removed once the fiber is resumed. With this approach, there's no need to reset the recheckCount since previous callback instance will be garbage collected once the resource is ready.

callback.proceedFromWait(callResponse.getResult());
return null;
}

if (shouldWait()) {
if ((callback.getRecheckCount() % RECHECK_DEBUG_COUNT) == 0) {
LOGGER.fine(LOG_WAITING_COUNT, serverName, callback.getRecheckCount());
}
// Watch backstop recheck count is less than or equal to the configured recheck count, delay.
return doDelay(createReadAndIfReadyCheckStep(callback), packet,
getWatchBackstopRecheckDelaySeconds(), TimeUnit.SECONDS);
} else {
LOGGER.fine(EXECUTE_MAKE_RIGHT_DOMAIN, serverName, callback.getRecheckCount());
removeCallback(resource, callback);
// Watch backstop recheck count is more than configured recheck count, proceed to make-right step.
return doNext(NEXT_STEP_FACTORY.createMakeDomainRightStep(callback, info, getNext()), packet);
}
}

private String getPodLabel(V1Pod pod) {
return Optional.ofNullable(pod)
.map(V1Pod::getMetadata)
.map(V1ObjectMeta::getLabels)
.map(m -> m.get(LabelConstants.SERVERNAME_LABEL))
.orElse(null);
}

private V1Pod getServerPod(DomainPresenceInfo info, String serverName) {
return Optional.ofNullable(serverName).map(info::getServerPod).orElse(null);
}

private boolean isNotFoundOnRead(CallResponse callResponse) {
return callResponse.getResult() == null;
}

private boolean shouldWait() {
return callback.incrementAndGetRecheckCount() <= getWatchBackstopRecheckCount();
}
};
}

}

private class WaitForPodReadyStep extends WaitForPodStatusStep {
Expand Down Expand Up @@ -360,13 +433,25 @@ protected void removeCallback(String podName, Consumer<V1Pod> callback) {
protected void logWaiting(String name) {
LOGGER.fine(MessageKeys.WAITING_FOR_POD_READY, name);
}

@Override
protected boolean onReadNotFoundForCachedResource(V1Pod cachedPod, boolean isNotFoundOnRead) {
// Return true if cached pod is not null but pod not found in explicit read, false otherwise.
return (cachedPod != null) && isNotFoundOnRead;
}

}

private class WaitForPodDeleteStep extends WaitForPodStatusStep {
private WaitForPodDeleteStep(V1Pod pod, Step next) {
super(pod, next);
}

@Override
protected boolean onReadNotFoundForCachedResource(V1Pod cachedPod, boolean isNotFoundOnRead) {
return false;
}

// A pod is considered deleted when reading its value from Kubernetes returns null.
@Override
protected boolean isReady(V1Pod result) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -194,17 +194,20 @@ class WatchTuning {
public final int watchLifetime;
public final int watchMinimumDelay;
public final int watchBackstopRecheckDelay;
public final int watchBackstopRecheckCount;

/**
* Create watch tuning.
* @param watchLifetime Watch lifetime
* @param watchMinimumDelay Minimum delay before accepting new events to prevent hot loops
* @param watchBackstopRecheckDelay Recheck delay for get while waiting for a status to backstop missed watch events
*/
public WatchTuning(int watchLifetime, int watchMinimumDelay, int watchBackstopRecheckDelay) {
public WatchTuning(int watchLifetime, int watchMinimumDelay, int watchBackstopRecheckDelay,
int watchBackstopRecheckCount) {
this.watchLifetime = watchLifetime;
this.watchMinimumDelay = watchMinimumDelay;
this.watchBackstopRecheckDelay = watchBackstopRecheckDelay;
this.watchBackstopRecheckCount = watchBackstopRecheckCount;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ private void update() {
new WatchTuning(
(int) readTuningParameter("watchLifetime", 300),
(int) readTuningParameter("watchMinimumDelay", 5),
(int) readTuningParameter("watchBackstopRecheckDelaySeconds", 5));
(int) readTuningParameter("watchBackstopRecheckDelaySeconds", 5),
(int) readTuningParameter("watchBackstopRecheckCount", 60));

PodTuning pod =
new PodTuning(
Expand Down
Loading