Skip to content

Conversation

@jackdelv
Copy link
Contributor

@jackdelv jackdelv commented Oct 15, 2025

  • Moves dir-per-part detection to addFile during initial scan
  • Publish final stats from scans after heartbeat ends
  • Remove stripe dir detection code that is no longer necessary
  • Turn off logs for every file/directory/part

The optimization marginally improved the runtime. I have included the timings because I though it was strange how the Directory scan phase sped up slightly despite the extra workload and the Orphan scan only slightly sped up despite it doing seemingly a lot less work.

  • With optimization = Remove mergeDirPerPart logic and add files to correct directory in addFile
  • Without optimization = Control test before changes were added
  • Less Logs + With optimization = After adding optimization, turned off logs for every part/file/directory
Screenshot 2025-10-15 144814

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:

  • My code follows the code style of this project.
    • My code does not create any new warnings from compiler, build system, or lint.
  • The commit message is properly formatted and free of typos.
    • The commit message title makes sense in a changelog, by itself.
    • The commit is signed.
  • My change requires a change to the documentation.
    • I have updated the documentation accordingly, or...
    • I have created a JIRA ticket to update the documentation.
    • Any new interfaces or exported functions are appropriately commented.
  • I have read the CONTRIBUTORS document.
  • The change has been fully tested:
    • I have added tests to cover my changes.
    • All new and existing tests passed.
    • I have checked that this change does not introduce memory leaks.
    • I have used Valgrind or similar tools to check for potential issues.
  • I have given due consideration to all of the following potential concerns:
    • Scalability
    • Performance
    • Security
    • Thread-safety
    • Cloud-compatibility
    • Premature optimization
    • Existing deployed queries will not be broken
    • This change fixes the problem, not just the symptom
    • The target branch of this pull request is appropriate for such a change.
  • There are no similar instances of the same problem that should be addressed
    • I have addressed them here
    • I have raised JIRA issues to address them separately
  • This is a user interface / front-end modification
    • I have tested my changes in multiple modern browsers
    • The component(s) render as expected

Smoketest:

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

Testing:

Copilot AI review requested due to automatic review settings October 15, 2025 19:33
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Optimizes directory scanning by moving dir-per-part file detection into addFile during initial scan, removing the later merge logic and reducing per-item logging. Adds final heartbeat completion statistics and suppresses previous debug logging blocks.

  • Moved dir-per-part detection from post-processing (mergeDirPerPartDirs) into on-the-fly file addition (getFile/addFile changes)
  • Added finishHeartbeat to emit aggregate stats and replaced end-of-scan messages
  • Removed mergeDirPerPartDirs logic and disabled several debug logging sections

@jackdelv
Copy link
Contributor Author

I'm not sure about the prepareFileAndReturnLock structure. Should the logic be left in addFile to avoid having to return a critical block? Should I break up the dir-per-part detection to make it clearer? Copilot seemed to have a lot of the same complaints for the new code, so I don't think it is currently the best way to approach it.

@jackdelv jackdelv requested a review from jakesmith October 15, 2025 19:54
@github-actions
Copy link

Jira Issue: https://hpccsystems.atlassian.net//browse/HPCC-35100

Jirabot Action Result:
Workflow Transition To: Merge Pending
Updated PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant