HPCC-30599 Fix file access costs for keyed join #18023

shamser · 2023-11-13T14:38:15Z

Type of change:

This change is a bug fix (non-breaking change which fixes an issue).
This change is a new feature (non-breaking change which adds functionality).
This change improves the code (refactor or other change that does not change the functionality)
This change fixes warnings (the fix does not alter the functionality or the generated code)
This change is a breaking change (fix or feature that will cause existing behavior to change).
This change alters the query API (existing queries will have to be recompiled)

Checklist:

Smoketest:

Send notifications about my Pull Request position in Smoketest queue.
Test my draft Pull Request.

Testing:

github-actions · 2023-11-13T14:38:54Z

https://track.hpccsystems.com/browse/HPCC-30599
Jira updated

system/jlib/jstats.cpp

jakesmith

@shamser - please see comments.

jakesmith · 2023-11-15T12:22:49Z

dali/base/dadfs.cpp

+extern da_decl cost_type calcFileAccessCost(IDistributedFile *f, __int64 numDiskWrites, __int64 numDiskReads)
+{
+    StringBuffer clusterName;
+    f->getClusterName(0, clusterName);


it may not be supported in containerized, but logical files can be on >1 cluster,
although only 1 will have been accessed..

Maybe we can gloss over this for now, but should add a comment here at least.

system/jhtree/jhtree.cpp

jakesmith · 2023-11-15T12:42:26Z

thorlcr/activities/indexread/thindexreadslave.cpp

@@ -350,17 +350,21 @@ class CIndexReadSlaveBase : public CSlaveActivity
    }
    void mergeFileStats(IPartDescriptor *partDesc, IFileIO *partIO)
    {
+        if (!currentManager)


mergeFileStats is only called when 'lazyIFileIO' is valid (from updateStats())
Which is called when configureNextInput is called (and in done()).
'currentManager' will always be null afaics (except in done()) at the moment.
It should be the outgoing IKeyManager for the the part it's just finished with but configureNextInput has set it to null before updateStats() is called.

So I don't think the updates can be working atm, but perhaps not noticed as long as the query finishes.

updateStats() probably need to change to keep currentManager until after the updateStats() call.

It also means (not new), that there are no fileStats updates serialized whilst it is reading a single part.
So for most index reads that means it won't send anything for fileStats until the end of the activity.
Needs revisiting in a separate JIRA.

thorlcr/activities/keyedjoin/thkeyedjoinslave.cpp

thorlcr/graph/thgraphmaster.cpp

thorlcr/activities/keyedjoin/thkeyedjoinslave.cpp

ecl/hthor/hthor.cpp

system/jlib/jstats.cpp

thorlcr/activities/indexread/thindexreadslave.cpp

jakesmith · 2023-11-23T11:13:52Z

thorlcr/thorutil/thormisc.cpp

@@ -91,6 +91,7 @@ const StatisticsMapping graphStatistics({StNumExecutions, StSizeSpillFile, StSiz
 const StatisticsMapping diskReadPartStatistics({StNumDiskRowsRead}, diskReadRemoteStatistics);
 const StatisticsMapping indexDistribActivityStatistics({}, basicActivityStatistics, jhtreeCacheStatistics);
 const StatisticsMapping soapcallActivityStatistics({}, basicActivityStatistics, soapcallStatistics);
+const StatisticsMapping indexReadFileStatistics({StNumRowsProcessed}, diskReadRemoteStatistics, jhtreeCacheStatistics);


worth reusing this, since it's a subset of indexReadActivityStatistics, i.e. move above it and have:

const StatisticsMapping indexReadActivityStatistics({}, basicActivityStatistics, indexReadFileStatistics);

jakesmith · 2023-11-23T12:20:22Z

thorlcr/activities/keyedjoin/thkeyedjoinslave.cpp

@@ -1535,16 +1527,28 @@ class CKeyedJoinSlave : public CSlaveActivity, implements IJoinProcessor, implem
        {
            handles.resize(parts.size());
            PARENT::init();
+            contextLoggers.clear();
+            for (unsigned i=0; i<parts.size(); i++)
+                contextLoggers.push_back(new CStatsContextLogger(jhtreeCacheStatistics, thorJob));


doesn't this mapping need to be indexReadFileStatistics?

no. Please see previous explanation https://github.com/hpcc-systems/HPCC-Platform/pull/18023/files#r1406087606

jakesmith · 2023-11-23T12:21:36Z

thorlcr/activities/keyedjoin/thkeyedjoinslave.cpp

        std::atomic<IKeyManager *> keyManager{nullptr};
    public:
-        CKeyLookupMergeHandler(CKeyedJoinSlave &_activity) : CKeyLookupLocalBase(_activity)
+        CKeyLookupMergeHandler(CKeyedJoinSlave &_activity) : CKeyLookupLocalBase(_activity), contextLogger(jhtreeCacheStatistics, thorJob)


does this mapping need to be a indexReadFileStatistics?

no. Please see previous explanation https://github.com/hpcc-systems/HPCC-Platform/pull/18023/files#r1406087606

jakesmith · 2023-11-23T12:25:16Z

thorlcr/activities/keyedjoin/thkeyedjoinslave.cpp

@@ -1367,6 +1368,9 @@ class CKeyedJoinSlave : public CSlaveActivity, implements IJoinProcessor, implem
            keyManagers.reset(new std::vector<std::atomic<IKeyManager *>>(parts.size()));
            for (auto & k: *keyManagers)
                k = nullptr;
+            contextLoggers.clear();
+            for (unsigned i=0; i<parts.size(); i++)
+                contextLoggers.push_back(new CStatsContextLogger(jhtreeCacheStatistics, thorJob));


doesn't this mapping need to be indexReadFileStatistics (same as fileStats) ?
(because they are used in the keymanager which is going to [try] to populate with io stats as well as jhtree stats)

no, contextLoggers will only ever tracking jhtree stats - they do not track io stats. We only need indexReadFileStatistics for fileStats. Note,

jhtree cache stats are returned CKeyLookupLocalHandler::getFileStats() and CRemoteLookupHandler::getFileStats, or

io stats are returned for CFetchLocalLookupHandler::getFileStats

recent change:

jhtree and io stats are returned from CKeyLookupLocalHandler::getFileStats(). It uses keyManager->mergeStats()

some jhtree and io stats are returned CRemoteLookupHandler::getFileStats().

io stats are returned from CFetchLocalLookupHandler::getFileStats

The original point is still valid: contextLoggers only track jhtree stats, so they don't need indexReadFileStatistics.

Is this related to the offline conversation we had, where the conclusion was that we don't currently collect and publish the io stats, either from the local handler, or from the remote - so we don't want to fix it 1 side now ?
But should fix in a subsequent PR so io stats are collected, and published whether using a local or remote handler.

I think this is still true.
i.e. because of this jhtree only mapping, we are no longer tracking disk io stats, although they IKeyManager::mergeStats is capable of returning them.

I think for this PR, that's okay, and probably preferable, becuase the remote handler never did serialize those stats back, so now it is consistent.

But a JIRA should be opened to revisit this so both handlers return io stats too.
Can you open one and link it to this JIRA?

jira: https://track.hpccsystems.com/browse/HPCC-31001

jakesmith · 2023-11-23T12:29:59Z

thorlcr/activities/keyedjoin/thkeyedjoinslave.cpp

    protected:
        rank_t lookupSlave = RANK_NULL;
        mptag_t replyTag = TAG_NULL;
        ICommunicator *comm = nullptr;
        std::vector<unsigned> handles;
+        // One context logger per part. (Extract stats for logical file by mapping part to logical file/subfile)
+        std::vector<Owned<CStatsContextLogger>> contextLoggers;


I think this should be moved out of CRemoteLookupHandler, into CKeyLookupRemoteHandler.

CRemoteLookupHandler is also the base of the remote fetch handler (CFetchRemoteLookupHandler).
Neiher the local or the remote fetch handlers use context loggers.

jakesmith · 2023-11-23T12:39:44Z

thorlcr/activities/keyedjoin/thkeyedjoinslave.cpp

@@ -3368,7 +3377,6 @@ class CKeyedJoinSlave : public CSlaveActivity, implements IJoinProcessor, implem
    }
    virtual void stop() override
    {
-        CStatsScopedDeltaUpdater scoped(statsUpdater);


The final stats will be collected and serialized via the serializeStats->gatherActiveStats when the subgraph is stopped, i.e. via standard mechanism.
fileStats at that point will get their final update (as will activeStats), and then be serialized.

I think it's all correct, just checking the logic.

jakesmith

@shamser - looks good in general.

I few minor issues only within KJ I think.

However, I think there's at least 1 change, that should be extracted out as an initial PR before this PR (see comment : #18023 (comment)).
That will help reduce the scope/size of the actual KJ change.

Also, it looks like there may be some double counting issues in index read, since there's only one contextLogger shared amongst parts (you may not notice unless the index read was reading >1 part, but it also looked like it was double counting IO stats.
I've opened a new JIRA re. creating some regression tests that grab stats and output them as results, to get some coverage of stat. changes:

Could you also update the PR and commit message, to say what changes have been made and why, e.g. the removal of the deltaUpdater, the introduction of a CStatsContextLogger per part, contained in the handlers etc, and the reasoning.
We may know the reasoning, but it would help others if description explained.

jakesmith · 2023-11-23T12:50:03Z

@shamser - hadn't noticed until now, but I see there are some regression tests failing that look relevant.

shamser · 2023-11-28T16:29:42Z

@jakesmith I've fixed the remote key lookup stats. jobstats shows consistent jhtreecache stats between local and remote.

shamser · 2023-11-28T16:32:29Z

@jakesmith this PR is based on https://track.hpccsystems.com/browse/HPCC-30911. The first 2 commits shown in github, is related to https://track.hpccsystems.com/browse/HPCC-30911.

jakesmith

@jakesmith this PR is based on https://track.hpccsystems.com/browse/HPCC-30911. The first 2 commits shown in github, is related to https://track.hpccsystems.com/browse/HPCC-30911.

I'll hold off reviewing this further until the PR for 30911 is merged.
Please rebase this PR and tag me, when that is merged.

jakesmith

@shamser - I haven't re-reviewed the keyedjoin changes yet, I had some comments on indexread changes,
and I think it would be best, unless there's a blocking reason not to, to split those changes into a separate PR.
Will discuss offline.

jakesmith · 2023-12-08T09:39:09Z

thorlcr/activities/indexread/thindexreadslave.cpp

@@ -98,7 +98,7 @@ class CIndexReadSlaveBase : public CSlaveActivity
            if (!keyManager)
                throw MakeActivityException(&activity, 0, "Callback attempting to read blob with no key manager - index being read remotely?");
            needsBlobCleaning = true;
-            return (byte *) keyManager->loadBlob(id, dummy, &activity.contextLogger);
+            return (byte *) keyManager->loadBlob(id, dummy, nullptr);


doesn't this mean blob stats won't be recorded at all?
The loadBlob implementation is not using stats.ctx.

Either this needs to pass in use the associated contextLogger[], or IKeyManager::loadBlob should use the one it has in its KeyStatsCollector.

jakesmith · 2023-12-08T09:43:08Z

thorlcr/activities/indexread/thindexreadslave.cpp

@@ -169,6 +169,7 @@ class CIndexReadSlaveBase : public CSlaveActivity
            unsigned p = partNum;
            while (p<partDescs.ordinality()) // will process all parts if localMerge
            {
+                CStatsContextLogger * contextLogger = contextLoggers[p];


minor: only used on line 308, would be clearer if declared near its usage, or contextLogger[p] used directly

indexreadslave changes will be moved to separate PR.

jakesmith · 2023-12-08T09:47:40Z

thorlcr/activities/indexread/thindexreadslave.cpp

@@ -331,7 +332,8 @@ class CIndexReadSlaveBase : public CSlaveActivity
                    return createIndexLookup(keyManager);
                }
            }
-            keyMergerManager.setown(createKeyMerger(helper->queryDiskRecordSize()->queryRecordAccessor(true), keyIndexSet, seekGEOffset, &contextLogger, helper->hasNewSegmentMonitors(), false));
+            //Not tracking jhtree cache stats in KeyMerger at the moment.  Future: something to consider


can you create a linked JIRA to revisit this?

indexreadslave changes will be moved to separate PR.

jakesmith · 2023-12-08T09:59:37Z

thorlcr/activities/indexread/thindexreadslave.cpp

+            ISuperFileDescriptor *superFDesc = partDescs.item(0).queryOwner().querySuperFileDescriptor();
+            for (unsigned partNum=0; partNum<partDescs.ordinality(); partNum++)
+            {
+                if (!keyManagers.isItem(partNum))


I am not sure this is thread safe.
getNextInput could be in the process of expanding the keyManagers array, isItem could be true, but entry still uninitialized.

An alternative may be to presize the keyManagers array (and initialize with nulls), and test for null rather than isItem

Similar comments change of approach, re. updateStats()

It used to be thread-safe, by locking a muxex (ioStatsCS), and dealing with just the current part.
I think it can still do the same, dealing with the currentManager only.

indexreadslave changes will be moved to separate PR.

jakesmith · 2023-12-08T13:20:23Z

common/thorhelper/thorcommon.hpp

@@ -762,6 +762,10 @@ class CStatsContextLogger : public CSimpleInterfaceOf<IContextLogger>
        previous.updateDelta(to, stats);
    }
    virtual const LogMsgJobInfo & queryJob() const override { return job; }
+    void reset()


style: we normally group declare the non-interface/non-virtuals. i.e. can this be moved after the ctor in this class.

jakesmith · 2023-12-08T13:30:58Z

thorlcr/activities/indexread/thindexreadslave.cpp

-        if (lazyIFileIO)
-        {
-            mergeStats(inactiveStats, lazyIFileIO);
-            if (currentPart<partDescs.ordinality())


I am not sure the approach here, should have been changed.
i.e. it used to roll the outgoing part into inactiveStats.
It used to just merge the current io, but can equally be applied to currentManager.

indexreadslave changes will be moved to separate PR.

jakesmith

@shamser - looks good, one question re. blob stats, and some minors + comments re. opening 'future' JIRAs

jakesmith · 2023-12-08T15:57:23Z

thorlcr/activities/keyedjoin/thkeyedjoinslave.cpp

@@ -1309,7 +1308,7 @@ class CKeyedJoinSlave : public CSlaveActivity, implements IJoinProcessor, implem
                        joinGroup->setAtMostLimitHit(); // also clears existing rows
                        break;
                    }
-                    KLBlobProviderAdapter adapter(keyManager, &activity.contextLogger);
+                    KLBlobProviderAdapter adapter(keyManager, nullptr);


how are blob stats. now being tracked ?

it looks like the remote side (slavmain) would return track and serialize blob stats.
Please test an example to verify same whether using local or remote. Regression test 'indexblobs.ecl' may work as a test query.

Blob stats are not being tracked for keyed join. However, the blob stats for keyed join was never correct because slavmain was not tracking blob stats. Also, hthor was not tracking blob stats. It would be better to address this in a separate jira as the changes affect not only thor but roxie and hthor too. TODO: create ticket once jira is working again.

jira: https://track.hpccsystems.com/browse/HPCC-31002

jakesmith · 2023-12-08T15:59:58Z

thorlcr/activities/keyedjoin/thkeyedjoinslave.cpp

@@ -1367,6 +1368,9 @@ class CKeyedJoinSlave : public CSlaveActivity, implements IJoinProcessor, implem
            keyManagers.reset(new std::vector<std::atomic<IKeyManager *>>(parts.size()));
            for (auto & k: *keyManagers)
                k = nullptr;
+            contextLoggers.clear();
+            for (unsigned i=0; i<parts.size(); i++)
+                contextLoggers.push_back(new CStatsContextLogger(jhtreeCacheStatistics, thorJob));


I think this is still true.
i.e. because of this jhtree only mapping, we are no longer tracking disk io stats, although they IKeyManager::mergeStats is capable of returning them.

I think for this PR, that's okay, and probably preferable, becuase the remote handler never did serialize those stats back, so now it is consistent.

But a JIRA should be opened to revisit this so both handlers return io stats too.
Can you open one and link it to this JIRA?

jakesmith · 2023-12-08T16:03:20Z

thorlcr/activities/keyedjoin/thkeyedjoinslave.cpp

@@ -2077,6 +2106,8 @@ class CKeyedJoinSlave : public CSlaveActivity, implements IJoinProcessor, implem
            PARENT::end();
            doClose(kjs_fetchclose);
        }
+        // Not tracking remote fetch file stats.  Future: may be something to consider.


can you open a JIRA and link it to this JIRA ?

https://track.hpccsystems.com/browse/HPCC-31000

jakesmith · 2023-12-08T16:04:29Z

thorlcr/graph/thgraphmaster.cpp

@@ -649,25 +649,48 @@ void CMasterActivity::done()

 void CMasterActivity::updateFileReadCostStats()
 {
-    // Updates numDiskReads & readCost in the file attributes and returns the readCost
-    auto updateReadCosts = [](IDistributedFile *file, CThorStatsCollection &stats)
+    // Returns updates numDiskReads & readCost in the file attributes and returns the readCost


was this comment accidentally reverted? (the one on deleted line 652 seems correct)

Yes, fixed now.

thorlcr/graph/thgraphmaster.cpp

jakesmith · 2023-12-08T16:32:11Z

thorlcr/activities/keyedjoin/thkeyedjoinslave.cpp

-            }
+            // CKeyLookupMergeHandler is not tracking the stats at the part level and that means it is not possible to associate the stats with a  logical file level.
+            // Superfile: all stats will be associated with the first sub index file.  For non-super, it makes no difference (there is only one index file).
+            // (Future: make necessary changes to createKeyMerger to track stats at the part level/logical level.)


can you open a JIRA and link it to this one?

https://track.hpccsystems.com/browse/HPCC-30999

jakesmith

@shamser - looks good, 1 question re. reset() of contextLogger's

jakesmith · 2023-12-13T12:51:41Z

thorlcr/activities/keyedjoin/thkeyedjoinslave.cpp

+        {
+            PARENT::reset();
+            for (auto contextLogger: contextLoggers)
+                contextLogger->reset();


is this correct? has it been tested?
i.e. what happens when this is called (because KJ is in a child query or loop), are the total stats correct?

jakesmith

@shamser - looks good. Please squash.

Do the remote vs local handler stats. (for index seek/scans) match for the CQ test query ?

thorlcr/slave/slavmain.cpp

shamser · 2023-12-15T17:00:28Z

@shamser - looks good. Please squash.

Do the remote vs local handler stats. (for index seek/scans) match for the CQ test query ?

seeks and scans match.

CacheHits is greater (202,062 vs 159499) on non-remote lookup. NumDiskReads is greater on non-remote (16 vs 12). Would you expect these to match?

jakesmith · 2023-12-18T14:42:19Z

CacheHits is greater (202,062 vs 159499) on non-remote lookup. NumDiskReads is greater on non-remote (16 vs 12). Would you expect these to match?

not sure either (both local and remote) are deterministic, because there could be multiple handlers or multiple parts being dealt with by the same process (with shared jhtree cache). However, if N part key, on a N-way Thor - then it may be deterministic..

I'm unsure, I think we can investigate these differences post this PR though.

jakesmith

@shamser - there are some build problems reported in the tests - I think needs rebasing.

jakesmith

@shamser - looks good.

ghalliday

@shamser looks good. A couple of questions - please could you answer them. Even if they require changes it is probably best to merge as-is and then revisit as separate PRs.

ghalliday · 2023-12-19T14:43:34Z

system/jhtree/jhtree.cpp

        if (keyCursor)
-            keyCursor->mergeStats(stats);
+            keyCursor->mergeStats(targetStats); // merge IO stats
+        if (activeCtx)


This doesn't feel like it is in the correct place. I would expect this call to be in the code that calls mergeStats():
as well as calling mergeStats on each keyManager it would call merge stats on the context's stats.

As it is this forces you to have a context for each key, which seems unnecessary, and wrong - e.g., in roxie it would only have a shared context, and prior to this change so did thor.

Probably best to merge and then revisit to avoid this being blocked.

Also, this should be testing ctx rather than activectx (otherwise if the last lookup was on a TLK that matched nothing it could avoid the merge.

I've changed to use ctx.

I agree that it does make sense to move the ctx mergeStats out to the caller. However, I'd prefer doing this separately so I can take the time to make the change and re-test.

Improvement: https://track.hpccsystems.com/browse/HPCC-31035

ghalliday · 2023-12-19T14:54:51Z

thorlcr/slave/slavmain.cpp

+
+                        CRuntimeStatisticCollection deltaStats(startStats.queryMapping());
+                        contextLogger.updateStatsDeltaTo(deltaStats, startStats);
+                        replyMb.append(deltaStats.getStatisticValue(StNumIndexSeeks));


Question: Would it be better to use deltaStats.serialize(replyMb)? Otherwise you are missing some of the stats.

Yes, it would be. I was thinking about doing this separately later as it was not related to kj costs.

Future: https://track.hpccsystems.com/browse/HPCC-29822

ghalliday · 2023-12-19T15:02:07Z

thorlcr/graph/thgraphmaster.cpp

+        stat_type curDiskReads = stats.getStatisticSum(StNumDiskReads);
+        if(useJhtreeCacheStats)
+        {
+            stat_type numActualReads = stats.getStatisticSum(StNumNodeDiskFetches)


Should this code be setting curDiskReads? It will affect the number of reads recorded against the file in line 675. (It would also allow the cost calculation to be shared.)

I recall @jakesmith mentioned that it would be useful to have both: the original NumDiskReads and the jhtree stats.

I think it is fine as it stands.
As it is, we have 2 pieces of info., we have the number of attempts to read from disk (StNumDiskReads) and the number of actual disk reads, at least in the index case (StNumNodeDiskFetches+StNumLeafDiskFetches+StNumBlobDiskFetches).

StNumDiskReads is a tad misleading (and always was), and may not closely correspond to actual disk reads in flat files in some cases either.
The explanation for what it is (and other stats) is something that should be clarified somewhere, but not directly related to this PR I would say.

Signed-off-by: Shamser Ahmed <[email protected]>

shamser commented Nov 13, 2023

View reviewed changes

system/jlib/jstats.cpp Outdated Show resolved Hide resolved

shamser force-pushed the issue30599 branch 2 times, most recently from bf1859f to 6fd995b Compare November 13, 2023 16:59

shamser marked this pull request as ready for review November 13, 2023 17:00

shamser requested a review from jakesmith November 13, 2023 17:00

shamser marked this pull request as draft November 14, 2023 13:05

jakesmith reviewed Nov 15, 2023

View reviewed changes

shamser force-pushed the issue30599 branch 7 times, most recently from a0916cb to c3b17fc Compare November 22, 2023 14:50

shamser marked this pull request as ready for review November 22, 2023 14:50

shamser requested a review from jakesmith November 22, 2023 14:50

jakesmith reviewed Nov 23, 2023

View reviewed changes

shamser force-pushed the issue30599 branch 7 times, most recently from 471e477 to 6df6af9 Compare November 28, 2023 16:25

shamser requested a review from jakesmith November 28, 2023 16:28

jakesmith mentioned this pull request Nov 29, 2023

HPCC-30911 Store read and write cost as file attributes #18080

Merged

39 tasks

jakesmith reviewed Dec 4, 2023

View reviewed changes

shamser force-pushed the issue30599 branch 2 times, most recently from ed64962 to 41fafa0 Compare December 7, 2023 17:39

shamser requested a review from jakesmith December 7, 2023 17:40

jakesmith reviewed Dec 8, 2023

View reviewed changes

shamser force-pushed the issue30599 branch from 41fafa0 to d8db4d3 Compare December 8, 2023 14:04

shamser requested a review from jakesmith December 8, 2023 14:14

jakesmith reviewed Dec 8, 2023

View reviewed changes

shamser force-pushed the issue30599 branch 2 times, most recently from 05935cf to 8681024 Compare December 11, 2023 11:19

shamser requested a review from jakesmith December 11, 2023 11:24

jakesmith reviewed Dec 13, 2023

View reviewed changes

shamser requested a review from jakesmith December 14, 2023 12:35

jakesmith reviewed Dec 14, 2023

View reviewed changes

thorlcr/slave/slavmain.cpp Outdated Show resolved Hide resolved

shamser force-pushed the issue30599 branch from 4ee77fd to 1aeffe8 Compare December 18, 2023 09:52

shamser requested a review from jakesmith December 18, 2023 09:52

shamser force-pushed the issue30599 branch from 1aeffe8 to f2781b1 Compare December 18, 2023 10:33

jakesmith reviewed Dec 18, 2023

View reviewed changes

shamser force-pushed the issue30599 branch from f2781b1 to bc4325a Compare December 19, 2023 11:12

shamser requested a review from jakesmith December 19, 2023 11:16

jakesmith approved these changes Dec 19, 2023

View reviewed changes

ghalliday reviewed Dec 19, 2023

View reviewed changes

shamser requested a review from ghalliday December 19, 2023 15:26

HPCC-30599 Fix file access costs for keyed join

461a8ae

Signed-off-by: Shamser Ahmed <[email protected]>

shamser force-pushed the issue30599 branch from 49e931e to 461a8ae Compare December 19, 2023 16:16

ghalliday approved these changes Dec 19, 2023

View reviewed changes

ghalliday merged commit 5d78149 into hpcc-systems:candidate-9.4.x Dec 19, 2023
21 checks passed

HPCC-30599 Fix file access costs for keyed join #18023

HPCC-30599 Fix file access costs for keyed join #18023

Conversation

shamser commented Nov 13, 2023 • edited Loading

Type of change:

Checklist:

Smoketest:

Testing:

github-actions bot commented Nov 13, 2023

jakesmith left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shamser Dec 11, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakesmith left a comment

Choose a reason for hiding this comment

jakesmith commented Nov 23, 2023

shamser commented Nov 28, 2023

shamser commented Nov 28, 2023

jakesmith left a comment

Choose a reason for hiding this comment

jakesmith left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakesmith left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shamser Dec 11, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shamser Dec 11, 2023 • edited Loading

Choose a reason for hiding this comment

jakesmith left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakesmith left a comment

Choose a reason for hiding this comment

shamser commented Dec 15, 2023

jakesmith commented Dec 18, 2023

jakesmith left a comment

Choose a reason for hiding this comment

jakesmith left a comment

Choose a reason for hiding this comment

ghalliday left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shamser Dec 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shamser commented Nov 13, 2023 •

edited

Loading

shamser Dec 11, 2023 •

edited

Loading

shamser Dec 11, 2023 •

edited

Loading

shamser Dec 11, 2023 •

edited

Loading

shamser Dec 19, 2023 •

edited

Loading