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-31028 Use cost_type internally for consistency #18154

Merged
merged 1 commit into from
Jan 11, 2024
Merged

HPCC-31028 Use cost_type internally for consistency #18154

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
24 changes: 12 additions & 12 deletions dali/base/dadfs.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,30 +191,30 @@ static IPropertyTree *getCostPropTree(const char *cluster)
}
}

extern da_decl double calcFileAtRestCost(const char * cluster, double sizeGB, double fileAgeDays)
extern da_decl cost_type calcFileAtRestCost(const char * cluster, double sizeGB, double fileAgeDays)
{
Owned<const IPropertyTree> costPT = getCostPropTree(cluster);

if (costPT==nullptr)
return 0.0;
return 0;
double atRestPrice = costPT->getPropReal("@storageAtRest", 0.0);
double storageCostDaily = atRestPrice * 12 / 365;
return storageCostDaily * sizeGB * fileAgeDays;
return money2cost_type(storageCostDaily * sizeGB * fileAgeDays);
}

extern da_decl double calcFileAccessCost(const char * cluster, __int64 numDiskWrites, __int64 numDiskReads)
extern da_decl cost_type calcFileAccessCost(const char * cluster, __int64 numDiskWrites, __int64 numDiskReads)
{
Owned<const IPropertyTree> costPT = getCostPropTree(cluster);

if (costPT==nullptr)
return 0.0;
return 0;
constexpr int accessPriceScalingFactor = 10000; // read/write pricing based on 10,000 operations
double readPrice = costPT->getPropReal("@storageReads", 0.0);
double writePrice = costPT->getPropReal("@storageWrites", 0.0);
return (readPrice * numDiskReads / accessPriceScalingFactor) + (writePrice * numDiskWrites / accessPriceScalingFactor);
return money2cost_type((readPrice * numDiskReads / accessPriceScalingFactor) + (writePrice * numDiskWrites / accessPriceScalingFactor));
}

extern da_decl double calcFileAccessCost(IDistributedFile *f, __int64 numDiskWrites, __int64 numDiskReads)
extern da_decl cost_type calcFileAccessCost(IDistributedFile *f, __int64 numDiskWrites, __int64 numDiskReads)
{
StringBuffer clusterName;
// Should really specify the cluster number too, but this is the best we can do for now
Expand Down Expand Up @@ -4942,7 +4942,7 @@ protected: friend class CDistributedFilePart;
else
return false;
}
virtual void getCost(const char * cluster, double & atRestCost, double & accessCost) override
virtual void getCost(const char * cluster, cost_type & atRestCost, cost_type & accessCost) override
{
CDateTime dt;
getModificationTime(dt);
Expand All @@ -4956,7 +4956,7 @@ protected: friend class CDistributedFilePart;
if (hasReadWriteCostFields(*attrs))
{
// Newer files have readCost and writeCost attributes
accessCost = cost_type2money(attrs->getPropInt64(getDFUQResultFieldName(DFUQRFreadCost)) + attrs->getPropInt64(getDFUQResultFieldName(DFUQRFwriteCost)));
accessCost = attrs->getPropInt64(getDFUQResultFieldName(DFUQRFreadCost)) + attrs->getPropInt64(getDFUQResultFieldName(DFUQRFwriteCost));
}
else
{
Expand Down Expand Up @@ -7041,12 +7041,12 @@ class CDistributedSuperFile: public CDistributedFileBase<IDistributedSuperFile>
return false;
}

virtual void getCost(const char * cluster, double & atRestCost, double & accessCost) override
virtual void getCost(const char * cluster, cost_type & atRestCost, cost_type & accessCost) override
{
CriticalBlock block (sect);
ForEachItemIn(i,subfiles)
{
double tmpAtRestcost, tmpAccessCost;
cost_type tmpAtRestcost, tmpAccessCost;
IDistributedFile &f = subfiles.item(i);
f.getCost(cluster, tmpAtRestcost, tmpAccessCost);
atRestCost += tmpAtRestcost;
Expand Down Expand Up @@ -13454,7 +13454,7 @@ IPropertyTreeIterator *deserializeFileAttrIterator(MemoryBuffer& mb, unsigned nu
else
sizeDiskSize = file->getPropInt64(getDFUQResultFieldName(DFUQRForigsize), 0);
double sizeGB = sizeDiskSize / ((double)1024 * 1024 * 1024);
cost_type atRestCost = money2cost_type(calcFileAtRestCost(nodeGroup, sizeGB, fileAgeDays));
cost_type atRestCost = calcFileAtRestCost(nodeGroup, sizeGB, fileAgeDays);
file->setPropInt64(getDFUQResultFieldName(DFUQRFatRestCost), atRestCost);

// Dyamically calc and set the access cost field and for legacy files set read/write cost fields
Expand Down
12 changes: 6 additions & 6 deletions dali/base/dadfs.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,7 @@ interface IDistributedFile: extends IInterface
virtual bool getSkewInfo(unsigned &maxSkew, unsigned &minSkew, unsigned &maxSkewPart, unsigned &minSkewPart, bool calculateIfMissing) = 0;
virtual int getExpire(StringBuffer *expirationDate) = 0;
virtual void setExpire(int expireDays) = 0;
virtual void getCost(const char * cluster, double & atRestCost, double & accessCost) = 0;
virtual void getCost(const char * cluster, cost_type & atRestCost, cost_type & accessCost) = 0;
};


Expand Down Expand Up @@ -889,9 +889,9 @@ extern da_decl void ensureFileScope(const CDfsLogicalFileName &dlfn, unsigned ti

extern da_decl bool checkLogicalName(const char *lfn,IUserDescriptor *user,bool readreq,bool createreq,bool allowquery,const char *specialnotallowedmsg);

extern da_decl double calcFileAtRestCost(const char * cluster, double sizeGB, double fileAgeDays);
extern da_decl double calcFileAccessCost(const char * cluster, __int64 numDiskWrites, __int64 numDiskReads);
extern da_decl double calcFileAccessCost(IDistributedFile *f, __int64 numDiskWrites, __int64 numDiskReads);
extern da_decl cost_type calcFileAtRestCost(const char * cluster, double sizeGB, double fileAgeDays);
extern da_decl cost_type calcFileAccessCost(const char * cluster, __int64 numDiskWrites, __int64 numDiskReads);
extern da_decl cost_type calcFileAccessCost(IDistributedFile *f, __int64 numDiskWrites, __int64 numDiskReads);
constexpr bool defaultPrivilegedUser = true;
constexpr bool defaultNonPrivilegedUser = false;

Expand All @@ -910,7 +910,7 @@ inline cost_type getLegacyReadCost(const IPropertyTree & fileAttr, Source source
&& !isFileKey(fileAttr))
{
stat_type prevDiskReads = fileAttr.getPropInt64(getDFUQResultFieldName(DFUQRFnumDiskReads), 0);
return money2cost_type(calcFileAccessCost(source, 0, prevDiskReads));
return calcFileAccessCost(source, 0, prevDiskReads);
}
else
return 0;
Expand All @@ -922,7 +922,7 @@ inline cost_type getLegacyWriteCost(const IPropertyTree & fileAttr, Source sourc
if (!hasReadWriteCostFields(fileAttr) && fileAttr.hasProp(getDFUQResultFieldName(DFUQRFnumDiskWrites)))
{
stat_type prevDiskWrites = fileAttr.getPropInt64(getDFUQResultFieldName(DFUQRFnumDiskWrites), 0);
return money2cost_type(calcFileAccessCost(source, prevDiskWrites, 0));
return calcFileAccessCost(source, prevDiskWrites, 0);
}
else
return 0;
Expand Down
6 changes: 3 additions & 3 deletions dali/daliadmin/daliadmin.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -726,10 +726,10 @@ static void testDFSFile(IDistributedFile *legacyDfsFile, const char *logicalName
PROGLOG("expire: %d", expire);
try
{
double atRestCost, accessCost;
cost_type atRestCost, accessCost;
legacyDfsFile->getCost(clusterName.str(), atRestCost, accessCost);
PROGLOG("accessCost: %f", accessCost);
PROGLOG("atRestCost: %f", atRestCost);
PROGLOG("accessCost: %f", cost_type2money(accessCost));
PROGLOG("atRestCost: %f", cost_type2money(atRestCost));
}
catch(IException *e)
{
Expand Down
2 changes: 1 addition & 1 deletion dali/dfu/dfurun.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ class CDFUengine: public CInterface, implements IDFUengine
DaftProgress::setRange(sizeReadBefore,totalSize,_totalNodes);
progress->setTotalNodes(_totalNodes);
}
void setFileAccessCost(double fileAccessCost)
void setFileAccessCost(cost_type fileAccessCost)
{
progress->setFileAccessCost(fileAccessCost);
}
Expand Down
6 changes: 3 additions & 3 deletions dali/dfu/dfuwu.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -779,7 +779,7 @@ class CDFUprogress: public CLinkedDFUWUchild, implements IDFUprogress
queryRoot()->getProp("@subdone",str);
return str;
}
double getFileAccessCost() const
cost_type getFileAccessCost() const
{
CriticalBlock block(parent->crit);
return queryRoot()->getPropInt64("@fileAccessCost");
Expand All @@ -795,10 +795,10 @@ class CDFUprogress: public CLinkedDFUWUchild, implements IDFUprogress
CriticalBlock block(parent->crit);
queryRoot()->setProp("@subdone",str);
}
void setFileAccessCost(double fileAccessCost)
void setFileAccessCost(cost_type fileAccessCost)
{
CriticalBlock block(parent->crit);
queryRoot()->setPropReal("@fileAccessCost", fileAccessCost);
queryRoot()->setPropInt64("@fileAccessCost", fileAccessCost);
Copy link
Member

Choose a reason for hiding this comment

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

Relaed to comment in ws_fsService.cpp.
The corresponding get function in this class is (unchanged):

    double getFileAccessCost() const
    {
        CriticalBlock block(parent->crit);
        return queryRoot()->getPropInt64("@fileAccessCost");
    }

that looks like a pre-existing bug. It was set as a real, but retrieved as an int (then cast to a double on return).

}
unsigned incPublisherTaskCount()
{
Expand Down
4 changes: 2 additions & 2 deletions dali/dfu/dfuwu.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ interface IConstDFUprogress: extends IInterface
virtual unsigned getTotalNodes() const = 0;
virtual StringBuffer &getSubInProgress(StringBuffer &str) const = 0; // sub-DFUWUs in progress
virtual StringBuffer &getSubDone(StringBuffer &str) const = 0; // sub-DFUWUs done (list)
virtual double getFileAccessCost() const = 0;
virtual cost_type getFileAccessCost() const = 0;
virtual unsigned getPublisherTaskCount() const = 0;
};

Expand All @@ -333,7 +333,7 @@ interface IDFUprogress: extends IConstDFUprogress
virtual void setSubInProgress(const char *str) = 0; // set sub-DFUWUs in progress
virtual void setSubDone(const char *str) = 0; // set sub-DFUWUs done
virtual void clearProgress() = 0;
virtual void setFileAccessCost(double fileAccessCost) = 0;
virtual void setFileAccessCost(cost_type fileAccessCost) = 0;
virtual unsigned incPublisherTaskCount() = 0;
};

Expand Down
2 changes: 1 addition & 1 deletion dali/ft/daft.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ interface IDaftProgress
{
virtual void onProgress(unsigned __int64 sizeDone, unsigned __int64 totalSize, unsigned numNodes, unsigned __int64 numReads, unsigned __int64 numWrites) = 0; // how much has been done
virtual void setRange(unsigned __int64 sizeReadBefore, unsigned __int64 totalSize, unsigned totalNodes) = 0; // how much has been done
virtual void setFileAccessCost(double fileAccessCost) = 0;
virtual void setFileAccessCost(cost_type fileAccessCost) = 0;
};

interface IDaftCopyProgress
Expand Down
4 changes: 2 additions & 2 deletions dali/ft/daftprogress.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class DALIFT_API DaftProgress : public IDaftProgress
unsigned numNodes, unsigned __int64 numReads, unsigned __int64 numWrites) = 0;
virtual void displaySummary(const char * timeTaken, unsigned kbPerSecond) = 0;
virtual void setRange(unsigned __int64 sizeReadBefore, unsigned __int64 totalSize, unsigned _totalNodes);
virtual void setFileAccessCost(double fileAccessCost) = 0;
virtual void setFileAccessCost(cost_type fileAccessCost) = 0;
protected:
void formatTime(char * buffer, unsigned secs);

Expand All @@ -59,7 +59,7 @@ class DALIFT_API DemoProgress : public DaftProgress
unsigned __int64 scaledDone, unsigned __int64 scaledTotal, const char * scale,
unsigned kbPerSecondAve, unsigned kbPerSecondRate, unsigned numNodes);
virtual void displaySummary(const char * timeTaken, unsigned kbPerSecond);
virtual void setFileAccessCost(double fileAccessCost) {};
virtual void setFileAccessCost(cost_type fileAccessCost) {};
};

#endif
Expand Down
6 changes: 3 additions & 3 deletions dali/ft/filecopy.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3387,7 +3387,7 @@ void FileSprayer::spray()
updateTargetProperties();

//Calculate and store file access cost
double fileAccessCost = 0.0;
cost_type fileAccessCost = 0;
if (distributedTarget)
{
StringBuffer cluster;
Expand Down Expand Up @@ -3592,7 +3592,7 @@ void FileSprayer::updateTargetProperties()

DistributedFilePropertyLock lock(distributedTarget);
IPropertyTree &curProps = lock.queryAttributes();
cost_type writeCost = money2cost_type(calcFileAccessCost(distributedTarget, totalNumWrites, 0));
cost_type writeCost = calcFileAccessCost(distributedTarget, totalNumWrites, 0);
curProps.setPropInt64(getDFUQResultFieldName(DFUQRFwriteCost), writeCost);
curProps.setPropInt64(getDFUQResultFieldName(DFUQRFnumDiskWrites), totalNumWrites);

Expand Down Expand Up @@ -3777,7 +3777,7 @@ void FileSprayer::updateTargetProperties()
{
IPropertyTree & fileAttr = distributedSource->queryAttributes();
cost_type legacyReadCost = getLegacyReadCost(fileAttr, distributedSource);
cost_type curReadCost = money2cost_type(calcFileAccessCost(distributedSource, 0, totalNumReads));
cost_type curReadCost = calcFileAccessCost(distributedSource, 0, totalNumReads);
distributedSource->addAttrValue(getDFUQResultFieldName(DFUQRFreadCost), legacyReadCost+curReadCost);
distributedSource->addAttrValue(getDFUQResultFieldName(DFUQRFnumDiskReads), totalNumReads);
}
Expand Down
6 changes: 3 additions & 3 deletions ecl/hthor/hthor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -789,7 +789,7 @@ void CHThorDiskWriteActivity::publish()
{
StringBuffer clusterName;
file->getClusterName(0, clusterName);
diskAccessCost = money2cost_type(calcFileAccessCost(clusterName, numDiskWrites, 0));
diskAccessCost = calcFileAccessCost(clusterName, numDiskWrites, 0);
properties.setPropInt64(getDFUQResultFieldName(DFUQRFwriteCost), diskAccessCost);
}
file->attach(logicalName.get(), agent.queryCodeContext()->queryUserDescriptor());
Expand Down Expand Up @@ -1437,7 +1437,7 @@ void CHThorIndexWriteActivity::execute()

StringBuffer clusterName;
dfile->getClusterName(0, clusterName);
diskAccessCost = money2cost_type(calcFileAccessCost(clusterName, numDiskWrites, 0));
diskAccessCost = calcFileAccessCost(clusterName, numDiskWrites, 0);
properties.setPropInt64(getDFUQResultFieldName(DFUQRFwriteCost), diskAccessCost);
}
else
Expand Down Expand Up @@ -8543,7 +8543,7 @@ void CHThorDiskReadBaseActivity::closepart()
}
IPropertyTree & fileAttr = dFile->queryAttributes();
cost_type legacyReadCost = getLegacyReadCost(fileAttr, dFile);
cost_type curReadCost = money2cost_type(calcFileAccessCost(dFile, 0, curDiskReads));
cost_type curReadCost = calcFileAccessCost(dFile, 0, curDiskReads);

dFile->addAttrValue(getDFUQResultFieldName(DFUQRFreadCost), legacyReadCost + curReadCost);
dFile->addAttrValue(getDFUQResultFieldName(DFUQRFnumDiskReads), curDiskReads);
Expand Down
2 changes: 1 addition & 1 deletion esp/clients/ws_dfsclient/ws_dfsclient.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ class CServiceDistributedFileBase : public CSimpleInterfaceOf<INTERFACE>
virtual bool isExternal() const override { return false; }
virtual bool getSkewInfo(unsigned &maxSkew, unsigned &minSkew, unsigned &maxSkewPart, unsigned &minSkewPart, bool calculateIfMissing) override { return legacyDFSFile->getSkewInfo(maxSkew, minSkew, maxSkewPart, minSkewPart, calculateIfMissing); }
virtual int getExpire(StringBuffer *expirationDate) override { return legacyDFSFile->getExpire(expirationDate); }
virtual void getCost(const char * cluster, double & atRestCost, double & accessCost) override { legacyDFSFile->getCost(cluster, atRestCost, accessCost); }
virtual void getCost(const char * cluster, cost_type & atRestCost, cost_type & accessCost) override { legacyDFSFile->getCost(cluster, atRestCost, accessCost); }

// setters (change file meta data)
virtual void setPreferredClusters(const char *clusters) override { legacyDFSFile->setPreferredClusters(clusters); }
Expand Down
8 changes: 4 additions & 4 deletions esp/services/ws_dfu/ws_dfuService.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2667,15 +2667,15 @@ void CWsDfuEx::doGetFileDetails(IEspContext &context, IUserDescriptor *udesc, co
}
if (version >= 1.60)
{
double atRestCost, accessCost;
cost_type atRestCost, accessCost;
df->getCost(cluster, atRestCost, accessCost);

if (version <= 1.61)
FileDetails.setCost(atRestCost+accessCost);
FileDetails.setCost(cost_type2money(atRestCost+accessCost));
else
{
FileDetails.setAccessCost(accessCost);
FileDetails.setAtRestCost(atRestCost);
FileDetails.setAccessCost(cost_type2money(accessCost));
FileDetails.setAtRestCost(cost_type2money(atRestCost));
}
}
if (version >= 1.65)
Expand Down
2 changes: 1 addition & 1 deletion esp/services/ws_fs/ws_fsService.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ static void DeepAssign(IEspContext &context, IConstDFUWorkUnit *src, IEspDFUWork
if(secs > 0)
dest.setSecsLeft(secs);
dest.setPercentDone(prog->getPercentDone());
dest.setFileAccessCost(prog->getFileAccessCost());
dest.setFileAccessCost(cost_type2money(prog->getFileAccessCost()));
Copy link
Member

Choose a reason for hiding this comment

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

IConstDFUprogress::getFileAccessCost is still returning a double.

}

IConstDFUoptions *options = src->queryOptions();
Expand Down
8 changes: 4 additions & 4 deletions thorlcr/graph/thgraphmaster.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -661,18 +661,18 @@ void CMasterActivity::updateFileReadCostStats()
{
// Legacy file: calculate readCost using prev disk reads and new disk reads
stat_type prevDiskReads = fileAttr.getPropInt64(getDFUQResultFieldName(DFUQRFnumDiskReads), 0);
legacyReadCost = money2cost_type(calcFileAccessCost(clusterName, 0, prevDiskReads));
legacyReadCost = calcFileAccessCost(clusterName, 0, prevDiskReads);
}
stat_type curDiskReads = stats.getStatisticSum(StNumDiskReads);
if(useJhtreeCacheStats)
{
stat_type numActualReads = stats.getStatisticSum(StNumNodeDiskFetches)
+ stats.getStatisticSum(StNumLeafDiskFetches)
+ stats.getStatisticSum(StNumBlobDiskFetches);
curReadCost = money2cost_type(calcFileAccessCost(clusterName, 0, numActualReads));
curReadCost = calcFileAccessCost(clusterName, 0, numActualReads);
}
else
curReadCost = money2cost_type(calcFileAccessCost(clusterName, 0, curDiskReads));
curReadCost = calcFileAccessCost(clusterName, 0, curDiskReads);
file->addAttrValue(getDFUQResultFieldName(DFUQRFreadCost), legacyReadCost + curReadCost);
file->addAttrValue(getDFUQResultFieldName(DFUQRFnumDiskReads), curDiskReads);
return curReadCost;
Expand Down Expand Up @@ -728,7 +728,7 @@ void CMasterActivity::updateFileWriteCostStats(IFileDescriptor & fileDesc, IProp
assertex(fileDesc.numClusters()>=1);
StringBuffer clusterName;
fileDesc.getClusterGroupName(0, clusterName);// Note: calculating for 1st cluster. (Future: calc for >1 clusters)
cost_type writeCost = money2cost_type(calcFileAccessCost(clusterName, numDiskWrites, 0));
cost_type writeCost = calcFileAccessCost(clusterName, numDiskWrites, 0);
props.setPropInt64(getDFUQResultFieldName(DFUQRFwriteCost), writeCost);
diskAccessCost = writeCost;
}
Expand Down
Loading