Generalized parallel processing logic #148
Conversation
Baliedge
changed the title
Baliedge
force-pushed
the
Baliedge/PIP-2552-consistent-ordering-3
branch
from
f727a94
to
9bff565
Compare
Baliedge
changed the title
Baliedge
force-pushed
the
Baliedge/PIP-2552-consistent-ordering-3
branch
3 times, most recently
from
53e3e31
to
8f710b2
Compare
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #148 +/- ##
========================================
Coverage 99.80% 99.80%
========================================
Files 148 149 +1
Lines 10673 10824 +151
========================================
+ Hits 10652 10803 +151
Misses 21 21
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
|
@daveshanley Ready for GHA re-run. |
|
There seems to be a flaky bit of code causing the tests to crash. It's caused by: https://github.com/pb33f/libopenapi/blob/main/datamodel/low/v3/paths_test.go#L122 It's crashing because circular detection is not being picked up and the stack is overflowing. https://github.com/pb33f/libopenapi/blob/main/datamodel/low/extraction_functions.go#L79 Suggestion to fix is find out why the loop is stacking up out of control / put in a hard limit on the depth extractions can go, just as a hard limit on a runaway mine train. Anything more than 100 levels deep is way out of control anyway. |
|
@daveshanley FYI, been stuck on other projects lately. Maybe able to find time next week to resolve these test coverage issues. |
I hear you man, no problem at all. Quality takes time. |
|
Some changes in my latest PR #162 should fix some of the issues causing the random failures. |
Baliedge
force-pushed
the
Baliedge/PIP-2552-consistent-ordering-3
branch
from
4960be8
to
df9f819
Compare
|
Just needs a tiny bit more coverage. Gotta keep it high. |
Baliedge
force-pushed
the
Baliedge/PIP-2552-consistent-ordering-3
branch
from
21bf650
to
6aed734
Compare
|
I am OK with the coverage, this looks ready to merge. Are you ready? |
|
@Baliedge is this complete? |
Yes, it's functionally complete. The codecov isn't passing for small portions of uncovered error handlers. I haven't had the opportunity to address that yet. |
|
@daveshanley Test coverage should now be passing. Please re-run the PR checks. |
…ize parallel processing of slices and channels in stable order.
Fix goroutine resource leak in `datamodel/low/v3/path_item.go`.
…ator. Integrate `TranslateMapParallel()` into datamodel for `Paths` to replace specialized async logic.
Integrate `TranslatePipeline()` into datamodel for schema components to replace specialized async logic.
Baliedge
force-pushed
the
Baliedge/PIP-2552-consistent-ordering-3
branch
from
edb5a35
to
f95f6b4
Compare
|
Which is less work to fix in a conflict, merging in #180 first, or this PR? |
|
Merging this first. |
Convert specialized parallel processing logic to generalized "translate" functions.
This is needed to prepare for significant datamodel changes supporting consistent ordering (#119) as the changes needed will refactor the same areas of code.
In several places, parallel processing has been implemented to improve performance. However, the logic is specialized and is prone to concurrency flaws without proper test coverage. Using a set of generalized functions will decrease the chance of introducing bugs when refactoring for consistent ordering. e.g. Want to be able to change the data type and container iteration logic without breaking channel logic.
Where possible, these translation functions will preserve input order.
Functions:
TranslateSliceParallel(): Iterates a slice, passes values through a translate function in parallel, then sends results sequentially in stable order to result function.TranslateMapParallel(): Iterates a map, passes value pairs through a translate function in parallel, then sends results sequentially to result function. Ordering is nondeterministic due to nature ofmapbuilt-in.TranslatePipeline(): Reads an input channel, passes values through a translate function in parallel, then sends results sequentially to output channel in stable order.