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

HPCC-30994 Ensure jobId's removed from log manager in worker #18128

Merged
merged 1 commit into from
Dec 14, 2023
Merged

HPCC-30994 Ensure jobId's removed from log manager in worker #18128

Show file tree
Hide file tree
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
1 change: 1 addition & 0 deletions thorlcr/slave/slavmain.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1885,6 +1885,7 @@ class CJobListener : public CSimpleInterface
ILogMsgFilter *existingLogHandler = queryLogMsgManager()->queryMonitorFilter(logHandler);
dbgassertex(existingLogHandler);
verifyex(queryLogMsgManager()->changeMonitorFilterOwn(logHandler, getCategoryLogMsgFilter(existingLogHandler->queryAudienceMask(), existingLogHandler->queryClassMask(), maxLogDetail)));
queryLogMsgManager()->removeJobId(thorJob.queryJobID());
LogMsgJobId thorJobId = queryLogMsgManager()->addJobId(wuid);
thorJob.setJobID(thorJobId);
setDefaultJobId(thorJobId);
Expand Down
53 changes: 29 additions & 24 deletions thorlcr/slave/thslavemain.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,24 +292,35 @@ class CReleaseMutex : public CSimpleInterface, public Mutex
ILogMsgHandler *startSlaveLog()
{
ILogMsgHandler *logHandler = nullptr;
#ifndef _CONTAINERIZED
StringBuffer fileName("thorslave");
Owned<IComponentLogFileCreator> lf = createComponentLogFileCreator(globals->queryProp("@logDir"), "thor");
StringBuffer slaveNumStr;
lf->setPostfix(slaveNumStr.append(mySlaveNum).str());
lf->setCreateAliasFile(false);
lf->setName(fileName.str());//override default filename
logHandler = lf->beginLogging();
if (!isContainerized())
{
StringBuffer fileName("thorslave");
Owned<IComponentLogFileCreator> lf = createComponentLogFileCreator(globals->queryProp("@logDir"), "thor");
StringBuffer slaveNumStr;
lf->setPostfix(slaveNumStr.append(mySlaveNum).str());
lf->setCreateAliasFile(false);
lf->setName(fileName.str());//override default filename
logHandler = lf->beginLogging();
#ifndef _DEBUG
// keep duplicate logging output to stderr to aide debugging
queryLogMsgManager()->removeMonitor(queryStderrLogMsgHandler());
// keep duplicate logging output to stderr to aide debugging
queryLogMsgManager()->removeMonitor(queryStderrLogMsgHandler());
#endif

LOG(MCdebugProgress, thorJob, "Opened log file %s", lf->queryLogFileSpec());
#else
setupContainerizedLogMsgHandler();
logHandler = queryStderrLogMsgHandler();
#endif
LOG(MCdebugProgress, thorJob, "Opened log file %s", lf->queryLogFileSpec());
}
else
{
setupContainerizedLogMsgHandler();
logHandler = queryStderrLogMsgHandler();
StringBuffer wuid;
if (getComponentConfigSP()->getProp("@workunit", wuid))
{
LogMsgJobId thorJobId = queryLogMsgManager()->addJobId(wuid);
thorJob.setJobID(thorJobId);
setDefaultJobId(thorJobId);
}
}

//setupContainerizedStorageLocations();
LOG(MCdebugProgress, thorJob, "Build %s", hpccBuildInfo.buildTag);
return logHandler;
Expand Down Expand Up @@ -385,12 +396,9 @@ int main( int argc, const char *argv[] )
usage();

mySlaveNum = globals->getPropInt("@slavenum", NotFound);
/* NB: in cloud/non-local storage mode, slave number is not known until after registration with the master
Copy link
Member

Choose a reason for hiding this comment

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

Is this comment no longer correct (I can't get to jira to check if you commented there).

Copy link
Member Author

Choose a reason for hiding this comment

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

this part of the comment would have still been true, 2nd line was misleading.
The comment is now replaced with a more explicit check and exception (line 399-400)

* For the time being log file names are based on their slave number, so can only start when known.
*/
ILogMsgHandler *slaveLogHandler = nullptr;
if (NotFound != mySlaveNum)
slaveLogHandler = startSlaveLog();
if (!isContainerized() && (NotFound == mySlaveNum))
throw makeStringException(0, "Slave number not specified (@slavenum)");
ILogMsgHandler *slaveLogHandler = startSlaveLog();

// In container world, SLAVE= will not be used
const char *slave = globals->queryProp("@slave");
Expand Down Expand Up @@ -427,9 +435,6 @@ int main( int argc, const char *argv[] )

if (RegisterSelf(masterEp))
{
if (!slaveLogHandler)
slaveLogHandler = startSlaveLog();

if (getExpertOptBool("slaveDaliClient"))
enableThorSlaveAsDaliClient();

Expand Down
Loading