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-33057 Allow attributes for implicit storage planes to be merged #19364

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

HPCC-33057 Allow attributes for implicit storage planes to be merged #19364

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8d4b2b4
HPCC-33057 Allow attributes for implicit storage planes to be merged
jackdelv Dec 16, 2024
24fb144
Check for mirror name first
jackdelv Mar 6, 2025
203efd5
Fix inverted test
jackdelv Mar 11, 2025
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
72 changes: 42 additions & 30 deletions dali/base/dafdesc.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3570,53 +3570,65 @@ bool GroupInformation::checkIsSubset(const GroupInformation & other)

void GroupInformation::createStoragePlane(IPropertyTree * storage, unsigned copy) const
{
// Check that storage plane does not already have a definition
VStringBuffer xpath("planes[@name='%s']", name.str());
if (storage->hasProp(xpath))
return;

IPropertyTree * plane = storage->addPropTree("planes");
StringBuffer mirrorname;
const char * planeName = name;
if (copy != 0)
planeName = mirrorname.append(name).append("_mirror");

plane->setProp("@name", planeName);
// Check that storage plane does not already have a definition
VStringBuffer xpath("planes[@name='%s']", planeName);
IPropertyTree * plane = storage->queryPropTree(xpath);
if (!plane)
{
plane = storage->addPropTree("planes");
plane->setProp("@name", planeName);
plane->setPropBool("@fromGroup", true);
}

//URL style drop zones don't generate a host entry, and will have a single device
if (ordinality() != 0)
{
if (container)
if (!plane->hasProp("@hostGroup"))
{
const char * containerName = container->name;
if (copy != 0)
containerName = mirrorname.clear().append(containerName).append("_mirror");
//hosts will be expanded by normalizeHostGroups
plane->setProp("@hostGroup", containerName);
}
else
{
//Host group has been created that matches the name of the storage plane
plane->setProp("@hostGroup", planeName);
if (container)
{
const char * containerName = container->name;
if (copy != 0)
containerName = mirrorname.clear().append(containerName).append("_mirror");
//hosts will be expanded by normalizeHostGroups
plane->setProp("@hostGroup", containerName);
}
else
{
//Host group has been created that matches the name of the storage plane
plane->setProp("@hostGroup", planeName);
}
}

if (ordinality() > 1)
if (!plane->hasProp("@numDevices"))
{
plane->setPropInt("@numDevices", ordinality());
if (dropZoneIndex == 0)
plane->setPropInt("@defaultSprayParts", ordinality());
if (ordinality() > 1)
{
plane->setPropInt("@numDevices", ordinality());
if (dropZoneIndex == 0)
plane->setPropInt("@defaultSprayParts", ordinality());
}
}
}

if (dir.length())
plane->setProp("@prefix", dir);
else
plane->setProp("@prefix", queryBaseDirectory(groupType, copy));

const char * category = (dropZoneIndex != 0) ? "lz" : "data";
plane->setProp("@category", category);
if (!plane->hasProp("@prefix"))
{
if (dir.length())
plane->setProp("@prefix", dir);
else
plane->setProp("@prefix", queryBaseDirectory(groupType, copy));
}

plane->setPropBool("@fromGroup", true);
if (!plane->hasProp("@category"))
{
const char * category = (dropZoneIndex != 0) ? "lz" : "data";
plane->setProp("@category", category);
}

//MORE: If container is identical to this except for the name we could generate an information tag @alias
}
Expand Down
5 changes: 3 additions & 2 deletions system/jlib/jfile.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7936,7 +7936,8 @@ MODULE_INIT(INIT_PRIORITY_STANDARD)
{
const IPropertyTree &plane = planesIter->query();
PlaneAttributesMapElement &element = planeAttributesMap[plane.queryProp("@name")];
element.first = plane.queryProp("@prefix");
const char * prefix = plane.queryProp("@prefix");
element.first = prefix ? prefix : ""; // If prefix is empty, avoid segfault by setting it to an empty string
auto &values = element.second;
for (unsigned propNum=0; propNum<PlaneAttributeType::PlaneAttributeCount; ++propNum)
{
Expand Down Expand Up @@ -8045,7 +8046,7 @@ static PlaneAttributesMapElement *findPlaneElementFromPath(const char *filePath)
for (auto &e: planeAttributesMap)
{
const char *prefix = e.second.first.c_str();
if (prefix) // sanity check, but should never be null
if (isEmptyString(prefix)) // sanity check, std::string cannot be null, so check if empty
Copy link
Member

Choose a reason for hiding this comment

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

Test has been inverted. Should be !isEmptyString()

Copy link
Member

Choose a reason for hiding this comment

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

@jakesmith we should have some unit tests to catch problems like this. Not quite sure how to code them easily though.

{
if (startsWith(filePath, prefix))
return &e.second;
Expand Down
Loading