Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RTI exit while saving the trace file #228

Merged
merged 8 commits into from
Jun 6, 2023
Merged

Conversation

ChadliaJerad
Copy link
Collaborator

This PR enables saving the trace file of the RTI, also after receiving a kill (Ctrl-C) signal.
This will help with debugging failing federated executions.

Copy link
Contributor

@edwardalee edwardalee left a comment

Choose a reason for hiding this comment

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

Some questions and suggestions. Thanks for fixing the printf format warnings too!

core/federated/RTI/rti.c Outdated Show resolved Hide resolved
core/federated/RTI/rti.c Outdated Show resolved Hide resolved
core/federated/RTI/rti_lib.h Outdated Show resolved Hide resolved
Copy link
Collaborator

@erlingrj erlingrj left a comment

Choose a reason for hiding this comment

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

Nice addition to the RTI :)

core/federated/RTI/rti.c Outdated Show resolved Hide resolved
core/federated/RTI/rti.c Outdated Show resolved Hide resolved
core/federated/RTI/rti_lib.h Outdated Show resolved Hide resolved
@ChadliaJerad
Copy link
Collaborator Author

@edwardalee and @erlingrj, I still don't get why the tests are not passing.
Can you please double-check?

@edwardalee
Copy link
Contributor

I have no idea why these tests are not passing. Looks like a socket connection was broken while downloading gradle? @petervdonovan : any ideas?

Copy link
Contributor

@edwardalee edwardalee left a comment

Choose a reason for hiding this comment

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

LGTM! I don't think the test failures have anything to do with these changes.

core/federated/RTI/rti.c Outdated Show resolved Hide resolved
@ChadliaJerad
Copy link
Collaborator Author

BTW, lf-gedf-np and lf-gedf-np-ci tests succeed in #224, which already has updates merged from rti-exit-save-trace (into rti-refactoring branch).

@petervdonovan
Copy link
Contributor

petervdonovan commented Jun 5, 2023

I have no idea why these tests are not passing. Looks like a socket connection was broken while downloading gradle? @petervdonovan : any ideas?

Three tests failed, all for different reasons:

  • I agree that there was a network issue while downloading Gradle. This isn't really our fault, although I suppose we could look into caching like what Marten implemented for Windows in lingua-franca. EDIT: It looks like the C tests already benefit from Gradle daemon brought down to free locks in Windows CI lingua-franca#1694, so there is nothing more we can do.
  • lf-gedf-np-ci tests are failing due to the bug that I pushed to master a few days ago. I expect the fix to be merged in the next couple of hours. (Fix bugs in name mangling lingua-franca#1816)
  • I don't know why the lf-adaptive tests failed, but it looks like some kind of flaky test failure related to federated communication. There isn't a lot of information to go on:
Federate DistributedCMakeIncludeSeparateCompile in Federation ID 'f6dd8f3f58e226751a9650be2bdc2372fd558d9f8a5271d5'
    #### Launching the federate federate__f1.
    #### Launching the federate federate__b1.
    #### Bringing the RTI back to foreground so it can receive Control-C.
    RTI -i ${FEDERATION_ID} -n 2 -c init exchanges-per-interval 10
    Federate 0: ERROR: Read 0 bytes, but expected 1. errno=57
    Federate 0: FATAL ERROR: Failed to read response from RTI.

EDIT: Raised an issue: lf-lang/lingua-franca#1819

EDIT: The bottom line is that I agree with Edward that the test failures are probably unrelated to these changes.

@petervdonovan petervdonovan merged commit 3444d87 into main Jun 6, 2023
@ChadliaJerad ChadliaJerad deleted the rti-exit-save-trace branch June 7, 2023 01:19
@lhstrh lhstrh added the enhancement Enhancement of existing feature label Aug 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement of existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants