search

epics-base / pvAccessCPP

pvAccessCPP is an EPICS V4 C++ module
https://epics-base.github.io/pvAccessCPP/
Other
10 stars 22 forks source link

clientContextImpl: Cap the number and age of beacons #191

Open JJL772 opened 1 year ago

JJL772 commented 1 year ago

Each beacon has an associated mutex. If we don't cap the beacon count, IOCs running on resource limited platforms like RTEMS may eventually run out of resources and crash.

Closes #184

The configuration options are probably unnecessary, let me know if I should remove them.

Requires some changes to pvData: https://github.com/epics-base/pvDataCPP/pull/94

mdavidsaver commented 1 year ago

Since it is not super obvious. The Beacon TX timing of pvAccessCPP differs from what experience with CA servers might lead you to expect.

https://github.com/epics-base/pvAccessCPP/blob/581d100a029e6f68f3c1830b90509b3ee5ebd7a0/src/server/beaconEmitter.cpp#L31-L33

A server will send out the first 10 beacons (not configurable) with a 15 second interval (by default), then switch to a 180 second period (not configurable). While $EPICS_PVAS_BEACON_PERIOD can override the first "fast" period, I don't think this is in practice useful.

So I think the beacon tracking lifetime must be >= 360 seconds.

(fyi. with PVXS I try to follow the same model and timings, with a non-configurable limit of 20k servers. Of course, there I only allocate ~64 bytes per server)

mdavidsaver commented 1 year ago

The windows CI failures are due to https://github.com/epics-base/ci-scripts/issues/84. When you update, please rebase to pick up ed7eae59bea24645e18562e5238e281fd12e5a99.

AppVeyorBot commented 1 year ago

:white_check_mark: Build pvAccessCPP 1.0.70 completed (commit https://github.com/epics-base/pvAccessCPP/commit/cdf3720715 by @JJL772)

JJL772 commented 11 months ago

@mdavidsaver Thanks for the feedback! I finally got around to applying the requested changes. I ended up using pvxs as a reference and copied the max beacon lifetime (360s) and beacon limit (20000).

AppVeyorBot commented 11 months ago

:x: Build pvAccessCPP 1.0.74 failed (commit https://github.com/epics-base/pvAccessCPP/commit/73c3932b45 by @JJL772)

AppVeyorBot commented 11 months ago

:x: Build pvAccessCPP 1.0.75 failed (commit https://github.com/epics-base/pvAccessCPP/commit/4e054f2e07 by @JJL772)

AppVeyorBot commented 11 months ago

:white_check_mark: Build pvAccessCPP 1.0.79 completed (commit https://github.com/epics-base/pvAccessCPP/commit/2ae88b70f1 by @JJL772)

JJL772 commented 9 months ago

@mdavidsaver Just wanted to follow up, are there any other changes required for this?

JJL772 commented 5 months ago

This PR now depends on some changes made to pvData: https://github.com/epics-base/pvDataCPP/pull/94

I'm going to mark this as a draft for now because I'm not exactly happy with these changes yet.

AppVeyorBot commented 5 months ago

:x: Build pvAccessCPP 1.0.108 failed (commit https://github.com/epics-base/pvAccessCPP/commit/c4e4658381 by @JJL772)

AppVeyorBot commented 5 months ago

:white_check_mark: Build pvAccessCPP 1.0.109 completed (commit https://github.com/epics-base/pvAccessCPP/commit/9651462441 by @JJL772)