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.

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

HPCC-32823 Add time waiting for git lock to the workunit stats #19250

Merged
merged 1 commit into from
Oct 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions ecl/eclcc/eclcc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1535,8 +1535,11 @@ void EclCC::processSingleQuery(const EclRepositoryManager & localRepositoryManag

updateWorkunitStat(instance.wu, SSToperation, ">compile:>parse", StTimeElapsed, NULL, parseTimeNs);
stat_type sourceDownloadTime = localRepositoryManager.getStatistic(StTimeElapsed);
stat_type sourceDownloadBlockedTime = localRepositoryManager.getStatistic(StTimeBlocked);
if (sourceDownloadTime)
updateWorkunitStat(instance.wu, SSToperation, ">compile:>parse:>download", StTimeElapsed, NULL, sourceDownloadTime);
if (sourceDownloadBlockedTime)
updateWorkunitStat(instance.wu, SSToperation, ">compile:>parse:>download", StTimeBlocked, NULL, sourceDownloadBlockedTime);

if (optExtraStats)
{
Expand Down
11 changes: 9 additions & 2 deletions ecl/hql/hqlrepository.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -676,6 +676,8 @@ unsigned __int64 EclRepositoryManager::getStatistic(StatisticKind kind) const
{
case StTimeElapsed:
return cycle_to_nanosec(gitDownloadCycles);
case StTimeBlocked:
return cycle_to_nanosec(gitDownloadBlockedCycles);
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure the context of when this getStatistic is called, but should these counters really atomic's ?

Copy link
Member Author

Choose a reason for hiding this comment

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

no, they are only called from a single thread.

}
return 0;
}
Expand Down Expand Up @@ -823,7 +825,12 @@ IEclSourceCollection * EclRepositoryManager::resolveGitCollection(const char * r
throw makeStringExceptionV(99, "Unsupported repository link format '%s'", defaultUrl);

bool alreadyExists = false;

CCycleTimer gitDownloadTimer;
Owned<IInterface> gitUpdateLock(getGitUpdateLock(repoPath));
cycle_t blockedCycles = gitDownloadTimer.elapsedCycles();
gitDownloadBlockedCycles += blockedCycles;

if (checkDirExists(repoPath))
{
if (options.cleanRepos)
Expand Down Expand Up @@ -853,7 +860,6 @@ IEclSourceCollection * EclRepositoryManager::resolveGitCollection(const char * r

bool ok = false;
Owned<IError> error;
CCycleTimer gitDownloadTimer;
if (alreadyExists)
{
if (options.updateRepos)
Expand Down Expand Up @@ -890,7 +896,8 @@ IEclSourceCollection * EclRepositoryManager::resolveGitCollection(const char * r
//this could become a read/write lock if that proved to be an issue.
gitUpdateLock.clear();

gitDownloadCycles += gitDownloadTimer.elapsedCycles();
gitDownloadCycles += (gitDownloadTimer.elapsedCycles() - blockedCycles);

if (error)
{
if (errorReceiver)
Expand Down
1 change: 1 addition & 0 deletions ecl/hql/hqlrepository.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ class HQL_API EclRepositoryManager
IArrayOf<IEclRepository> overrideSources; // -D options
IArrayOf<IEclRepository> allSources; // also includes -D options
cycle_t gitDownloadCycles = 0;
cycle_t gitDownloadBlockedCycles = 0;

//Include all options in a nested struct to make it easy to ensure they are cloned
struct {
Expand Down
Loading