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

Merge Fixes for physical test #45

Draft
wants to merge 25 commits into
base: master
Choose a base branch
from
Draft

Merge Fixes for physical test #45

wants to merge 25 commits into from

Conversation

BLuedtke
Copy link
Collaborator

@BLuedtke BLuedtke commented Dec 5, 2024

Closes #44

Currently a draft as we have to actually run these improved physical tests still.

Note: there's a fix contained in that branch for historical reasons, which is about a compiler warning wrt. a potentially uninitialized var.

@BLuedtke BLuedtke self-assigned this Dec 5, 2024
testsuite_stopBidib();
printf("testsuite: SIGINT - stopping libbidib \n");
printf("testsuite: SIG %d - libbidib stopped.\n", signum);
Copy link
Member

Choose a reason for hiding this comment

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

Instead of printing the signal code, you could use strsignal or sys_signame to print the signal name: https://stackoverflow.com/a/16509837

point_check &= testsuite_check_point_aspect("point12", "normal");
point_check &= testsuite_check_point_aspect("point10", "reverse");
point_check &= testsuite_check_point_aspect("point9", "reverse");
point_check &= testsuite_check_point_aspect("point11", "reverse");
Copy link
Member

Choose a reason for hiding this comment

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

Seems like a helper function should be defined that does the switching and checking of points: it takes a list of points and a corresponding list of aspects, it calls testsuite_switch_point on each pair of point-aspect, it sleeps for POINT_WAITING_TIME_S seconds, and then it calls testsuite_check_point_aspect on each pair of point-aspect.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Absolutely. I've done something like that when I implemented the physical tests in libbidib-cpp.
The reason I haven't done it here is that I don't really want to malloc all that stuff; and whilst it is technically possible to do it just statically (as the strings are all known already), I don't remember how to do that correctly when arrays of strings are involved, from a syntax standpoint 😅 .

Copy link
Collaborator Author

@BLuedtke BLuedtke Dec 6, 2024

Choose a reason for hiding this comment

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

Oh. It's much easier than I expected! Good news. As there are only 2 aspects, that part doesnt even need to be a parameter.

@eyip002
Copy link
Member

eyip002 commented Dec 5, 2024

I know that this is just a draft, but I left a few comments

@BLuedtke
Copy link
Collaborator Author

BLuedtke commented Dec 6, 2024

I finally found something wrt. the temporary lockups. And that is: the systemd journald SyncIntervalSec. The default is 5 minutes. If I set it 20 seconds and restart, I see problems/delays roughly every 20s. So maybe this is it? Syncing logs to disc being the bottleneck would not surprise me, given that this is syncing to an SD-card.

Based on the configuration options described in here, I'll test some optimizations while keeping the sync interval to 20s.

  • turning off compression and sealing to reduce effort per sync: complete disaster, everything is extremely delayed
  • turning off persistent storage completely (storage=volatile): works very reliably, no freezes at all. Syslog also seems to be more responsive. tested >10 runs. 🥳 🥳
    • note that this does not disable the syslog from being persisted to a file. It just disables the periodic journal from being persisted, this journal seems to include all syslog messages, and is apparently much more expensive to save than the usual syslog logs. IMO this can be turned off no problem.

Regarding the experimental observer I added, it is working well (with no noticable overhead). However, now that the freezes are pretty much gone, its unclear if its really needed anymore. I'll probably keep it and add the observing to the other parts of the physical test.

@eyip002
Copy link
Member

eyip002 commented Dec 6, 2024

  • turning off persistent storage completely (storage=volatile): works very reliably, no freezes at all. Syslog also seems to be more responsive. tested >10 runs. 🥳 🥳

Nice find!

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.

Fix physical tests
3 participants