Skip to content

Commit

Permalink
dcache-resilience: fix bug in source handling with Clear Cache Locati…
Browse files Browse the repository at this point in the history
…on messages

Motivation:

When a PnfsClearCacheLocationMessage arrives and file attribute
locations are not empty, Resilience needs to see if it can
react or repair the situation.  However, it must not consider
the pool reported in the message (obviously) as a valid source
for file replication.   There is a mistake in the code which
allows this to happen.

Modification:

Fix it so that the pool does not become the source file
of the operation.

Result:

No unnecessary Migration Task exceptions resulting from
source pools with no replica in the repository.

Target: master
Request: 4.0
Request: 3.2
Request: 3.1
Request: 3.0
Request: 2.16
Require-notes: yes
Require-book: no
Acked-by: Paul
  • Loading branch information
alrossi committed Feb 12, 2018
1 parent 736aa1b commit efe0d91
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -769,7 +769,7 @@ public boolean register(FileUpdate data) {
data.getSelectionAction(),
data.getCount(),
data.getSize());
operation.setParentOrSource(data.getPoolIndex(), data.isParent);
operation.setParentOrSource(data.getSourceIndex(), data.isParent());
operation.setVerifySticky(data.shouldVerifySticky());
FileAttributes attributes = data.getAttributes();
operation.setRetentionPolicy(attributes.getRetentionPolicy().toString());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,11 @@ LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
import org.dcache.resilience.util.LocationSelector;
import org.dcache.vehicles.FileAttributes;

import static org.dcache.resilience.data.MessageType.ADD_CACHE_LOCATION;
import static org.dcache.resilience.data.MessageType.CLEAR_CACHE_LOCATION;
import static org.dcache.resilience.data.MessageType.CORRUPT_FILE;
import static org.dcache.resilience.data.MessageType.POOL_STATUS_DOWN;

/**
* <p>A transient encapsulation of pertinent configuration data regarding
* a file location.</p>
Expand Down Expand Up @@ -124,7 +129,7 @@ public static FileAttributes getAttributes(PnfsId pnfsId, String pool,
}

if (attributes.getLocations().isEmpty()) {
if (messageType == MessageType.CLEAR_CACHE_LOCATION) {
if (messageType == CLEAR_CACHE_LOCATION) {
LOGGER.trace("ClearCacheLocationMessage for {}; "
+ "no current locations; "
+ "file probably deleted "
Expand All @@ -133,7 +138,7 @@ public static FileAttributes getAttributes(PnfsId pnfsId, String pool,
return null;
}

if (messageType != MessageType.ADD_CACHE_LOCATION) {
if (messageType != ADD_CACHE_LOCATION) {
/*
* Since the scan began or the broken file reported,
* the file has been removed.
Expand Down Expand Up @@ -174,8 +179,6 @@ public static FileAttributes getAttributes(PnfsId pnfsId, String pool,
public final MessageType type;
public final SelectionAction action;

final boolean isParent;

private final boolean isFullScan;

private Integer group;
Expand Down Expand Up @@ -220,18 +223,6 @@ public FileUpdate(PnfsId pnfsId, String pool, MessageType type,
this.pnfsId = pnfsId;
this.pool = pool;
this.type = type;
if (type == null) {
isParent = false;
} else {
switch (type) {
case POOL_STATUS_DOWN:
case POOL_STATUS_UP:
isParent = true;
break;
default:
isParent = false;
}
}
this.action = action;
this.group = group;
fromReload = false;
Expand Down Expand Up @@ -264,10 +255,19 @@ public Integer getUnitIndex() {
return unitIndex;
}

public Integer getSourceIndex() {
return type == CORRUPT_FILE ||
type == CLEAR_CACHE_LOCATION ? null : poolIndex;
}

public boolean isFromReload() {
return fromReload;
}

public boolean isParent() {
return type == POOL_STATUS_DOWN || type == POOL_STATUS_DOWN;
}

public void setCount(Integer count) {
this.count = count;
}
Expand All @@ -277,14 +277,16 @@ public void setFromReload(boolean fromReload) {
}

public boolean shouldVerifySticky() {
return !isFromReload() && type != MessageType.CLEAR_CACHE_LOCATION &&
(!isParent || action == SelectionAction.ADD);
return !isFromReload() && type != CLEAR_CACHE_LOCATION &&
(!isParent() || action == SelectionAction.ADD);
}

public String toString() {
return String.format(
"(%s)(%s)(%s)(parent %s)(psu action %s)(group %s)(count %s)",
pnfsId, pool, type, isParent, action, group, count);
"(%s)(%s)(%s)(parent %s)(source %s)"
+ "(psu action %s)(group %s)(count %s)",
pnfsId, pool, type, isParent(), getSourceIndex(),
action, group, count);
}

public boolean validateAttributes(NamespaceAccess access)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,8 @@ public Task handleMakeOneCopy(FileAttributes attributes) {
ReplicaState.CACHED, ONLINE_STICKY_RECORD,
Collections.EMPTY_LIST, attributes,
attributes.getAccessTime());
LOGGER.trace("Created migration task for {}: {}.", pnfsId, task);
LOGGER.trace("Created migration task for {}: source {}, list {}.",
pnfsId, source, list);

return task;
}
Expand Down

0 comments on commit efe0d91

Please sign in to comment.