mattcasters commented on PR #8850:
URL: https://github.com/apache/hop/pull/8850#issuecomment-6101218064
Thanks @bamaer for the thorough review!
I've addressed both the blocker and performance feedback in the latest
commit:
1. **Database Child Matching (Blocker)**:
- In `CachingDatabaseExecutionInfoLocation.loadCacheEntryWithChild()`:
- Updated the SQL query to exclude the child ID itself (`COL_ID != ?`),
order by `COL_EXECUTION_START_DATE DESC`, and limit to 20 candidate rows.
- Switched from `getOneRow()` to an `openQuery()` loop that inspects
candidates and verifies that `entry.peekChildExecution(childId) != null` or
`entry.getChildIds().contains(childId)`. Only valid parent entries are accepted
and cached.
- In `BaseCachingExecutionInfoLocation.getExecution()`:
- When `!entry.getId().equals(executionId)`, it now returns
`entry.getChildExecution(executionId)` directly instead of falling through to
the parent execution, returning `null` when the requested child execution does
not exist.
- Added `testUncachedParentChildLinkResolution` to
`CachingDatabaseExecutionInfoLocationTest` verifying parent-child resolution
(`Workflow` -> `Action` -> `Pipeline`) across uncached database entries and
asserting `assertNull(location.getExecution("non-existent-id"))`.
2. **File Scan Optimization (Should Fix)**:
- In `CachingFileExecutionInfoLocation.loadCacheEntryWithChild()`:
- Bounded the scan to the most recent files (`Math.min(fileList.size(),
Math.max(MAX_CHILD_SEARCH_SCAN, maxSize * 2))`, with `MAX_CHILD_SEARCH_SCAN =
100`).
- Cached file timestamps in a local record (`FileWithTimestamp`) during
the initial pass to avoid $O(N \log N)$ redundant VFS metadata calls during
sorting.
- Wrapped file processing in a `try ... finally` block that guarantees
all `FileObject` instances are closed via `closeQuietly()`.
- Read and parsed `CacheEntry` directly from the already-fetched file
content string in memory instead of re-reading the file from disk via
`loadCacheEntry(id)`.
- Added `assertNull(location.getExecution("non-existent-id"))`
assertion in `CachingFileExecutionInfoLocationTest`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
