Safe handling of control plane promises & fix CI #391

dagardner-nv · 2023-09-18T21:53:48Z

Description

Based on fixes from @mdemoret-nv : https://github.com/mdemoret-nv/MRC/tree/mdd_control-plane-promises & Improve service shutdown #381
Adopts updated versions of boost, clang & IWYU
Only run IWYU on files changed in PR
Use clang for checks stage
Allocate progress engine promises on the heap

fixes #379

Checklist

I am familiar with the Contributing Guidelines.
New or existing tests cover these changes.
The documentation is up to date with these changes.

…-cap-add=sys_nice [no ci]

…ntrol-plane-promises-dg

This reverts commit 5747320.

This reverts commit 29d9f00.

This reverts commit 5cf17e5.

This reverts commit 8524d89.

This reverts commit bd740aa.

This reverts commit c226ec1.

This reverts commit 1b20886.

… into mdd_control-plane-promises-dg-16

… they called executor.stop() insread of run

mdemoret-nv

Looks good. Mostly cleanup and fixing a few commented out tests

ci/scripts/github/checks.sh

cpp/mrc/src/internal/control_plane/client.cpp

cpp/mrc/src/internal/control_plane/client.hpp

cpp/mrc/src/internal/grpc/client_streaming.hpp

cpp/mrc/src/internal/grpc/promise_handler.cpp

cpp/mrc/tests/modules/test_segment_modules.cpp

cpp/mrc/tests/modules/test_stream_buffer_modules.cpp

mdemoret-nv

A few more changes from @cwharris review of #381 which contained most of these changes to the service class

cpp/mrc/src/internal/service.hpp

cpp/mrc/src/internal/service.cpp

cpp/mrc/src/internal/service.hpp

cpp/mrc/src/internal/service.cpp

Co-authored-by: Michael Demoret <42954918+mdemoret-nv@users.noreply.github.com>

…s, but CI checks against release builds

cpp/mrc/src/internal/service.cpp

codecov · 2023-09-22T23:01:20Z

Codecov Report

Merging #391 (a7fffce) into branch-23.11 (9b47924) will increase coverage by 0.20%.
Report is 8 commits behind head on branch-23.11.
The diff coverage is 86.82%.

Additional details and impacted files

@@               Coverage Diff                @@
##           branch-23.11     #391      +/-   ##
================================================
+ Coverage         73.11%   73.32%   +0.20%     
================================================
  Files               382      385       +3     
  Lines             13403    13671     +268     
  Branches           1010     1026      +16     
================================================
+ Hits               9800    10024     +224     
- Misses             3603     3647      +44

Flag	Coverage Δ
cpp	`68.03% <86.82%> (-1.02%)`	⬇️
py	`41.93% <31.32%> (-0.13%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed	Coverage Δ
...mrc/src/internal/control_plane/client/instance.cpp	`37.20% <ø> (ø)`
...rnal/control_plane/client/subscription_service.cpp	`0.00% <ø> (ø)`
cpp/mrc/src/internal/control_plane/server.hpp	`100.00% <ø> (ø)`
...c/src/internal/data_plane/data_plane_resources.cpp	`72.50% <ø> (ø)`
cpp/mrc/src/internal/data_plane/server.cpp	`55.65% <ø> (ø)`
.../mrc/src/internal/executor/executor_definition.cpp	`78.43% <ø> (-0.42%)`	⬇️
cpp/mrc/src/internal/pipeline/manager.cpp	`86.95% <ø> (ø)`
cpp/mrc/src/internal/remote_descriptor/manager.cpp	`63.06% <ø> (ø)`
cpp/mrc/src/internal/service.hpp	`0.00% <0.00%> (ø)`
cpp/mrc/src/tests/test_pipeline.cpp	`97.58% <ø> (-0.02%)`	⬇️
... and 21 more

... and 4 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fa5e40c...a7fffce. Read the comment docs.

dagardner-nv · 2023-09-23T00:17:08Z

/merge

PR #391 updated the version of boost, but I forgot to update the version in the package's yaml. Authors: - David Gardner (https://github.com/dagardner-nv) Approvers: - Devin Robison (https://github.com/drobison00) - Michael Demoret (https://github.com/mdemoret-nv) URL: #404

mdemoret-nv and others added 30 commits September 7, 2023 13:03

Updating control plane client to maintain promise lifetimes

1fad5eb

Removing Service constructor with description

44da13f

use the size_t specialization of atomic to match other usage of size_t

5a642aa

Allocate promises on the heap

8524d89

Incorporating updates from MNMG that make services more stable

74f988b

Getting tests to pass

34b4166

Add missing flags to docker launch to mount the working dir and set -…

2c72555

…-cap-add=sys_nice [no ci]

Merge branch 'branch-23.11' of github.com:nv-morpheus/MRC into mdd_co…

201f861

…ntrol-plane-promises-dg

Revert "Revert boost upgrade, and update clang to v16 (nv-morpheus#382)"

6671019

This reverts commit 5747320.

Re-work PromiseHandler as a component

29d9f00

Crash is no longer happening

913cc12

Revert "Re-work PromiseHandler as a component"

5cf17e5

This reverts commit 29d9f00.

Revert "Revert "Re-work PromiseHandler as a component""

c7f6939

This reverts commit 5cf17e5.

Revert "Allocate promises on the heap"

0e29298

This reverts commit 8524d89.

Add test viariant locked to singl core

89449b4

IWYU fixes

7732039

Make do_writes_done a public method

1b20886

Add do_writes_done method

c226ec1

wip

bd740aa

Revert "wip"

a37366a

This reverts commit bd740aa.

Revert "Add do_writes_done method"

76c3ef2

This reverts commit c226ec1.

Revert "Make do_writes_done a public method"

7a277cb

This reverts commit 1b20886.

Adopt clang-16

632235f

Only run iwyu on compilation units actually changed in PR

7b363f0

IWYU fix

c10f1ba

Merge branch 'mdd_fix-service-shutdown' of github.com:mdemoret-nv/MRC…

1491afd

… into mdd_control-plane-promises-dg-16

IWYU fixes

d143276

Comment out two broken tests, which previously never executed because…

235af51

… they called executor.stop() insread of run

Comment out two broken tests, which previously never executed because…

f32ac0b

… they called executor.stop() insread of run

Comment out unused includes

78606be

mdemoret-nv approved these changes Sep 21, 2023

View reviewed changes

mdemoret-nv mentioned this pull request Sep 21, 2023

Improve service shutdown #381

Closed

3 tasks

dagardner-nv added 5 commits September 21, 2023 10:34

Uncomment and skip broken tests [no ci]

f01b110

wip [no ci]

d9b0a2c

Move imple for non-template AsyncEventStatus methods to cpp file [no ci]

cdb2830

Remove unused mutex attribute [no ci]

6267039

Remove out of date commented code [no ci]

71f343d

mdemoret-nv reviewed Sep 21, 2023

View reviewed changes

dagardner-nv and others added 9 commits September 22, 2023 10:11

WIP: PR feedback [no ci]

551aed2

Update cpp/mrc/src/internal/service.cpp

eac3fb0

Co-authored-by: Michael Demoret <42954918+mdemoret-nv@users.noreply.github.com>

Update cpp/mrc/src/internal/service.cpp

81e777c

Co-authored-by: Michael Demoret <42954918+mdemoret-nv@users.noreply.github.com>

Update cpp/mrc/src/internal/service.hpp

65ee19d

Co-authored-by: Michael Demoret <42954918+mdemoret-nv@users.noreply.github.com>

Update cpp/mrc/src/internal/service.hpp

cdb9681

Co-authored-by: Michael Demoret <42954918+mdemoret-nv@users.noreply.github.com>

Update cpp/mrc/src/internal/service.cpp

d06824d

Co-authored-by: Michael Demoret <42954918+mdemoret-nv@users.noreply.github.com>

Update cpp/mrc/src/internal/service.cpp

4cf52b1

Co-authored-by: Michael Demoret <42954918+mdemoret-nv@users.noreply.github.com>

Remove unneeded scope

9835adb

Remove unused variable, and apply PR feedback

fb763fc

dagardner-nv requested a review from cwharris September 22, 2023 17:36

dagardner-nv added 3 commits September 22, 2023 10:55

Remove unused include

d9853ac

Add keep for mrc/utils/string_utils.hpp, we need this for debug build…

a1d76e3

…s, but CI checks against release builds

clang-tidy suggestions

10f8618

cwharris approved these changes Sep 22, 2023

View reviewed changes

cpp/mrc/src/internal/service.cpp Outdated Show resolved Hide resolved

Rename arg not the method

a7fffce

rapids-bot bot merged commit 40d20a3 into nv-morpheus:branch-23.11 Sep 23, 2023
14 checks passed

dagardner-nv deleted the mdd_control-plane-promises-dg-16-fix-shutdown-heap branch September 23, 2023 00:17

dagardner-nv mentioned this pull request Oct 3, 2023

Update boost versions to match version used in dev env #404

Merged

3 tasks

dagardner-nv mentioned this pull request Jan 26, 2024

[BUG]: Fix TestSegmentModules.ModuleInitializationTest & TestStreamBufferModule.InitailizationTest #390

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Safe handling of control plane promises & fix CI #391

Safe handling of control plane promises & fix CI #391

dagardner-nv commented Sep 18, 2023 •

edited

Loading

mdemoret-nv left a comment

mdemoret-nv left a comment

codecov bot commented Sep 22, 2023

dagardner-nv commented Sep 23, 2023

Safe handling of control plane promises & fix CI #391

Safe handling of control plane promises & fix CI #391

Conversation

dagardner-nv commented Sep 18, 2023 • edited Loading

Description

Checklist

mdemoret-nv left a comment

Choose a reason for hiding this comment

mdemoret-nv left a comment

Choose a reason for hiding this comment

codecov bot commented Sep 22, 2023

Codecov Report

dagardner-nv commented Sep 23, 2023

dagardner-nv commented Sep 18, 2023 •

edited

Loading