Skip to content

BigScreen - Fixed retrieval of latest available QO #2081

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

Merged
Barthelemy merged 1 commit into from
Jan 9, 2024
Merged

BigScreen - Fixed retrieval of latest available QO #2081

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
27 changes: 18 additions & 9 deletions Modules/Common/src/BigScreen.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "QualityControl/MonitorObject.h"
#include "QualityControl/DatabaseInterface.h"
#include "QualityControl/ObjectMetadataKeys.h"
#include "QualityControl/ActivityHelpers.h"
#include <TDatime.h>
#include <TPaveText.h>
#include <chrono>
Expand Down Expand Up @@ -105,19 +106,27 @@ void BigScreen::initialize(quality_control::postprocessing::Trigger t, framework

static std::pair<std::shared_ptr<QualityObject>, bool> getQO(repository::DatabaseInterface& qcdb, Trigger t, BigScreenConfig::DataSource& source, long notOlderThan, bool ignoreActivity)
{
// retrieve MO from CCDB - do not associate to trigger activity if ignoreActivity is true
auto qo = ignoreActivity ? qcdb.retrieveQO(source.path, t.timestamp, {}) : qcdb.retrieveQO(source.path, t.timestamp, t.activity);
if (!qo) {
// find the time-stamp of the most recent object matching the current activity
// if ignoreActivity is true the activity matching criteria are not applied
Activity activity = ignoreActivity ? Activity{} : t.activity;
auto timestamp = t.timestamp;
const auto objFullPath = t.activity.mProvenance + "/" + source.path;
const auto filterMetadata = activity_helpers::asDatabaseMetadata(activity, false);
const auto objectValidity = qcdb.getLatestObjectValidity(objFullPath, filterMetadata);
if (objectValidity.isValid()) {
timestamp = objectValidity.getMax() - 1;
Copy link
Copy Markdown
Collaborator

@knopers8 knopers8 Jan 8, 2024

Choose a reason for hiding this comment

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

why objectValidity.getMax() - 1 in particular?

Copy link
Copy Markdown
Contributor Author

@aferrero2707 aferrero2707 Jan 9, 2024

Choose a reason for hiding this comment

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

@knopers8 honestly I simply used the same logic as in the newObject trigger for the object's time stamp (see here).
If there is a better choice just let me know. Thanks!

Copy link
Copy Markdown
Collaborator

@knopers8 knopers8 Jan 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks. There, max-1 is used because it is the max timestamp that one can use to access the object. In the case of the trigger, I use it because it makes the most sense as a timestamp of data points on trending plots.

In your case, I am wondering if using "Created-At" instead of "Valid-Until" - 1 could make more sense. If we use the first, you will consider an object as too old if it was not updated recently. If you use the latter, a regularly updated object, but with no new data since some time (max validity does not move) will not be considered as too old will be considered as too old as well. Personally, I am OK with both alternatives, so I think it's up to you.

Copy link
Copy Markdown
Contributor Author

@aferrero2707 aferrero2707 Jan 9, 2024

Choose a reason for hiding this comment

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

For the purpose of the BigScreen, I think it is useful to also identify the cases where an object is regularly updated but no new data is actually received, and show it as grey if the last new data is too far in the past. What do you think?

Copy link
Copy Markdown
Collaborator

@knopers8 knopers8 Jan 10, 2024

Choose a reason for hiding this comment

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

Yes, indeed, this is something that shifters should be aware of.

So the algorithm would be:

  1. get the latest object validity
  2. if now - created > x, consider the object as invalid, use an appropriate colour in the display
  3. if now - validity_end > y consider the object as invalid, use an appropriate (different) colour in the display.

I would keep x and y as different parameters in case we want to tune them separately. After all, there is always some latency between the validity end and the object publication time.

Colour could be also different between the two, so it is clear which of the two cases we encounter.

Copy link
Copy Markdown
Contributor Author

@aferrero2707 aferrero2707 Jan 10, 2024

Choose a reason for hiding this comment

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

That's a very good suggestion! We can extend the logic in a separate PR, for example:

  • grey color if now - created > x (the processing is not creating new QOs)
  • yellow color if now - validity_end > y (processing creates new QOs, but from the same old data)

} else {
ILOG(Warning, Devel) << "Could not find an object '" << objFullPath << "' for activity " << activity << ENDM;
return { nullptr, false };
}
// get the MO creation time stamp
long timeStamp{ 0 };
auto iter = qo->getMetadataMap().find(repository::metadata_keys::created);
if (iter != qo->getMetadataMap().end()) {
timeStamp = std::stol(iter->second);

// retrieve QO from CCDB - do not associate to trigger activity if ignoreActivity is true
auto qo = qcdb.retrieveQO(source.path, timestamp, activity);
if (!qo) {
return { nullptr, false };
}

long elapsed = static_cast<long>(t.timestamp) - timeStamp;
long elapsed = static_cast<long>(t.timestamp) - timestamp;
// check if the object is not older than a given number of milliseconds
if (elapsed > notOlderThan) {
return { qo, false };
Expand Down