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

HPCC-11496 Avoid create/delete ClusterLock to reduce contention #5925

Merged
merged 1 commit into from May 22, 2014
Merged

HPCC-11496 Avoid create/delete ClusterLock to reduce contention #5925

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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 14 additions & 5 deletions dali/base/dadfs.cpp
View file
Open in desktop
Expand Up @@ -579,11 +579,13 @@ class CClustersLockedSection
{
StringBuffer xpath;
dlfn.makeFullnameQuery(xpath,DXB_File,true).append("/ClusterLock");
conn.setown(querySDS().connect(xpath.str(), myProcessSession(), RTM_CREATE | RTM_LOCK_WRITE | RTM_DELETE_ON_DISCONNECT, SDS_CONNECT_TIMEOUT));
}

~CClustersLockedSection()
{
/* Avoid RTM_CREATE_QUERY connect() if possible by making 1st call without. This is to avoid write contention caused by RTM_CREATE*
* NB: RTM_CREATE_QUERY should probably only gain exclusive access in Dali if node is missing.
*/
conn.setown(querySDS().connect(xpath.str(), myProcessSession(), RTM_LOCK_WRITE, SDS_CONNECT_TIMEOUT));
if (!conn.get()) // NB: ClusterLock is now created at File create time, so this can only be true for pre-existing File's
conn.setown(querySDS().connect(xpath.str(), myProcessSession(), RTM_CREATE_QUERY | RTM_LOCK_WRITE, SDS_CONNECT_TIMEOUT));
Copy link
Member

Choose a reason for hiding this comment

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

A comment explaining why we split this into two calls would be useful...

Copy link
Member

Choose a reason for hiding this comment

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

Or else fix behaviour of RTM_CREATE_QUERY so only gets exclusive access if it needs to?

Copy link
Member Author

Choose a reason for hiding this comment

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

I suspect it should, but I am more nervous about changing code in that area to switch the readwrite lock from a read to write conditionally.

Copy link
Member

Choose a reason for hiding this comment

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

As it stands, locking will temporarily be slower (until the ClusterLocks are created everywhere...)

Copy link
Member Author

Choose a reason for hiding this comment

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

That is true, until populated (which new files will be) or this creates it will be marginally slower (the 1st call will be not be contended, unlike the previous single call though)

Copy link
Member Author

Choose a reason for hiding this comment

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

It won't be slower than before though..
because the previous incarnation was not only blocking to create, but creating a commit to delete too..

}
};

Expand Down Expand Up @@ -3192,6 +3194,7 @@ protected: friend class CDistributedFilePart;
#endif
parent = _parent;
root.setown(createPTree(queryDfsXmlBranchName(DXB_File)));
root->setPropTree("ClusterLock", createPTree());
// fdesc->serializeTree(*root,IFDSF_EXCLUDE_NODES);
setFileAttrs(fdesc,true);
setClusters(fdesc);
Expand Down Expand Up @@ -7969,7 +7972,13 @@ void CDistributedFileDirectory::addEntry(CDfsLogicalFileName &dlfn,IPropertyTree
}
root->setProp("@name",tail.str());
root->setProp("OrigName",dlfn.get());
sroot->addPropTree(superfile?queryDfsXmlBranchName(DXB_SuperFile):queryDfsXmlBranchName(DXB_File),root); // now owns root
if (superfile)
sroot->addPropTree(queryDfsXmlBranchName(DXB_SuperFile), root); // now owns root
else
{
IPropertyTree *file = sroot->addPropTree(queryDfsXmlBranchName(DXB_File), root); // now owns root
file->setPropTree("ClusterLock", createPTree());
}
}

IDistributedFileIterator *CDistributedFileDirectory::getIterator(const char *wildname, bool includesuper, IUserDescriptor *user)
Expand Down