Skip to content

Commit

Permalink
pool: fix CDC for repository listener notification
Browse files Browse the repository at this point in the history
Motivation:

The repository supports the registration of listeners that
may learn of changes to replicas within the repository in a
decoupled fashion.

The CDC (logging context) for this notification is wrong, as the
notification happens on a separate, reused thread (from an executor) but
without establishing the correct context.  The result is any logging
will be recorded against arbitrary (and misleading) context, making
diagnosing problems much harder.

Modification:

Ensure the CDC is initialised correctly before calling the listeners,
using the caller's context.

Result:

Fix some logging on the pool where messages were recorded against an
arbitrary context (i.e., the bit in square brackets), resulting in
misleading information.

Target: master
Requires-notes: yes
Requires-book: no
Request: 5.0
Request: 4.2
Request: 4.1
Request: 4.0
Request: 3.2
Patch: https://rb.dcache.org/r/11589/
Acked-by: Tigran Mkrtchyan
  • Loading branch information
paulmillar committed Mar 6, 2019
1 parent d2488fa commit a663aad
Showing 1 changed file with 47 additions and 36 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
import java.util.concurrent.RejectedExecutionException;
import java.util.concurrent.TimeUnit;

import dmg.cells.nucleus.CDC;

import org.dcache.pool.repository.EntryChangeEvent;
import org.dcache.pool.repository.StateChangeEvent;
import org.dcache.pool.repository.StateChangeListener;
Expand Down Expand Up @@ -58,20 +60,23 @@ public void remove(StateChangeListener listener)

public void stateChanged(final StateChangeEvent event)
{
CDC callingContext = new CDC();
try {
_executor.execute(() -> {
for (StateChangeListener listener: _listeners) {
try {
listener.stateChanged(event);
} catch (RuntimeException e) {
/* State change notifications are
* important for proper functioning of the
* pool and we cannot risk a problem in an
* event handler causing other event
* handlers not to be called. We therefore
* catch, log and ignore these problems.
*/
_log.error("Unexpected failure during state change notification", e);
try (CDC oldContext = callingContext.restore()) {
for (StateChangeListener listener: _listeners) {
try {
listener.stateChanged(event);
} catch (RuntimeException e) {
/* State change notifications are
* important for proper functioning of the
* pool and we cannot risk a problem in an
* event handler causing other event
* handlers not to be called. We therefore
* catch, log and ignore these problems.
*/
_log.error("Unexpected failure during state change notification", e);
}
}
}
});
Expand All @@ -83,20 +88,23 @@ public void stateChanged(final StateChangeEvent event)

public void accessTimeChanged(final EntryChangeEvent event)
{
CDC callingContext = new CDC();
try {
_executor.execute(() -> {
for (StateChangeListener listener: _listeners) {
try {
listener.accessTimeChanged(event);
} catch (RuntimeException e) {
/* State change notifications are
* important for proper functioning of the
* pool and we cannot risk a problem in an
* event handler causing other event
* handlers not to be called. We therefore
* catch, log and ignore these problems.
*/
_log.error("Unexpected failure during state change notification", e);
try (CDC oldContext = callingContext.restore()) {
for (StateChangeListener listener: _listeners) {
try {
listener.accessTimeChanged(event);
} catch (RuntimeException e) {
/* State change notifications are
* important for proper functioning of the
* pool and we cannot risk a problem in an
* event handler causing other event
* handlers not to be called. We therefore
* catch, log and ignore these problems.
*/
_log.error("Unexpected failure during state change notification", e);
}
}
}
});
Expand All @@ -108,20 +116,23 @@ public void accessTimeChanged(final EntryChangeEvent event)

public void stickyChanged(final StickyChangeEvent event)
{
CDC callingContext = new CDC();
try {
_executor.execute(() -> {
for (StateChangeListener listener: _listeners) {
try {
listener.stickyChanged(event);
} catch (RuntimeException e) {
/* State change notifications are
* important for proper functioning of the
* pool and we cannot risk a problem in an
* event handler causing other event
* handlers not to be called. We therefore
* catch, log and ignore these problems.
*/
_log.error("Unexpected failure during state change notification", e);
try (CDC oldContext = callingContext.restore()) {
for (StateChangeListener listener: _listeners) {
try {
listener.stickyChanged(event);
} catch (RuntimeException e) {
/* State change notifications are
* important for proper functioning of the
* pool and we cannot risk a problem in an
* event handler causing other event
* handlers not to be called. We therefore
* catch, log and ignore these problems.
*/
_log.error("Unexpected failure during state change notification", e);
}
}
}
});
Expand Down

0 comments on commit a663aad

Please sign in to comment.