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.

Sign up for GitHub

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

Jump to bottom

Fix harvester execution logs added to previous logs #8387

Merged
merged 1 commit into from
Sep 26, 2024
Merged

Fix harvester execution logs added to previous logs #8387

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 3 additions & 3 deletions core/src/main/java/org/fao/geonet/util/LogUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2001-2023 Food and Agriculture Organization of the
* Copyright (C) 2001-2024 Food and Agriculture Organization of the
* United Nations (FAO-UN), United Nations World Food Programme (WFP)
* and United Nations Environment Programme (UNEP)
*
Expand Down Expand Up @@ -57,7 +57,7 @@ public static String initializeHarvesterLog(String type, String name) {
// Filename safe representation of harvester name (using '_' as needed).
final String harvesterName = name.replaceAll("\\W+", "_");
final String harvesterType = type.replaceAll("\\W+", "_");
SimpleDateFormat dateFormat = new SimpleDateFormat("yyyyMMddHHmm");
SimpleDateFormat dateFormat = new SimpleDateFormat("yyyyMMddHHmmss");

String logfile = "harvester_"
+ harvesterType
Expand All @@ -71,7 +71,7 @@ public static String initializeHarvesterLog(String type, String name) {
}

ThreadContext.put("harvest", harvesterName);
ThreadContext.putIfNull("logfile", logfile);
ThreadContext.put("logfile", logfile);
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks this makes sense if the thread is being recycled.

Q: Do we have a deinit method we could use to clear this setting?
Q: Is it worth doing some small check if this value is non-null and adding a System.out or Stacktrace to identify the conflict?

Copy link
Member Author

Choose a reason for hiding this comment

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

@jodygarnett

Q: Do we have a deinit method we could use to clear this setting?

Maybe can be added in the finally clause of

core-geonetwork/harvesters/src/main/java/org/fao/geonet/kernel/harvest/harvester/AbstractHarvester.java

Lines 597 to 687 in 835a293

protected OperResult harvest() {
OperResult operResult = OperResult.OK;
Boolean releaseLock = false;
try {
if (lock.isHeldByCurrentThread() || (releaseLock = lock.tryLock(LONG_WAIT, TimeUnit.SECONDS))) {
long startTime = System.currentTimeMillis();
running = true;
cancelMonitor.set(false);
try {
String logfile = LogUtil.initializeHarvesterLog(getType(), this.getParams().getName());
this.log.info("Starting harvesting of " + this.getParams().getName());
error = null;
errors.clear();
final Logger logger = this.log;
final String nodeName = getParams().getName() + " (" + getClass().getSimpleName() + ")";
final String lastRun = OffsetDateTime.now(ZoneOffset.UTC).format(DateTimeFormatter.ISO_DATE_TIME);
try {
login();
//--- update lastRun
harvesterSettingsManager.setValue("harvesting/id:" + id + "/info/lastRun", lastRun);
//--- proper harvesting
logger.info("Started harvesting from node : " + nodeName);
HarvestWithIndexProcessor h = new HarvestWithIndexProcessor(dataMan, logger);
// todo check (was: processwithfastindexing)
h.process(settingManager.getSiteId());
logger.info("Ended harvesting from node : " + nodeName);
if (getParams().isOneRunOnly()) {
stop(Status.INACTIVE);
}
} catch (InvalidParameterValueEx e) {
logger.error("The harvester " + this.getParams().getName() + "["
+ this.getType()
+ "] didn't accept some of the parameters sent.");
errors.add(new HarvestError(context, e));
error = e;
operResult = OperResult.ERROR;
} catch (UnknownHostException e) {
logger.error("The harvester " + this.getParams().getName() + "["
+ this.getType()
+ "] host is unknown.");
// Using custom UnknownHostEx as UnknownHostException error message differs from IPv4 / IPv6,
// in IPv4 only reports the host name in the message, the harvester uses this field
// and error reported to the user is unclear
error = new UnknownHostEx(e.getMessage());
errors.add(new HarvestError(context, error));
operResult = OperResult.ERROR;
} catch (Throwable t) {
operResult = OperResult.ERROR;
logger.warning("Raised exception while harvesting from : " + nodeName);
logger.warning(" (C) Class : " + t.getClass().getSimpleName());
logger.warning(" (C) Message : " + t.getMessage());
logger.error(t);
error = t;
errors.add(new HarvestError(context, t));
} finally {
List<HarvestError> harvesterErrors = getErrors();
if (harvesterErrors != null) {
errors.addAll(harvesterErrors);
}
}
long elapsedTime = TimeUnit.MILLISECONDS.toSeconds(System.currentTimeMillis() - startTime);
logHarvest(logfile, logger, nodeName, lastRun, elapsedTime);
} finally {
cancelMonitor.set(false);
running = false;
}
} else {
log.error("Harvester '" + this.getID() + "' looks deadlocked.");
log.error("Harvester '" + this.getID() + "' hasn't initiated.");
operResult = OperResult.ERROR;
}
} catch (InterruptedException e) {
log.error(e);
} finally {
if (lock.isHeldByCurrentThread() && releaseLock) {
lock.unlock();
}
}
return operResult;
a call to ThreadContext.clearAll()?

Q: Is it worth doing some small check if this value is non-null and adding a System.out or Stacktrace to identify the conflict?

Please try to add that to the original code and check the test case provided to confirm that the problem is that the information is in the ThreadContext of a reused thread.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay I understand this is urgent, and address the current problem.

The feedback above can be considered for a future improvement or if the problem reoccurs.

ThreadContext.put("timeZone", timeZoneSetting);

return logfile;
Expand Down
Loading