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

Shorten default shm_monitoring_domain to avoid path length problems o… #1166

Merged
merged 1 commit into from
Aug 2, 2023

Conversation

KerstinKeller
Copy link
Contributor

@KerstinKeller KerstinKeller commented Jul 27, 2023

…n MacOS.

Please check the type of change your PR introduces:

  • Bugfix

Cherry-Pick to:

  • 5.12

@FlorianReimold
Copy link
Member

Is that enough? If it is, then this will obviously get my approval

@hannemn
Copy link
Contributor

hannemn commented Jul 27, 2023

Only as a remark: This option only influences the name prefix of the shared memory files that are used for shared memory monitoring and has no effect on the file names where the acutual payloads become tranfered.

@FlorianReimold
Copy link
Member

Yes, but as I understood, that also was the only issue, right? we already reduced the path length for the payload and event SHM files some time ago

@KerstinKeller
Copy link
Contributor Author

@hannemn are you sure? From the code it seems that every opened memory file for the monitoring will hold the prefix.
Eg it tried to open ecal_monitoring_1394252728 (some ultraling hash) resulting in 33 or 36 chars. Shortening to ecal_mon then gives the a length <31 chars

@hannemn
Copy link
Contributor

hannemn commented Jul 31, 2023

Okay, I tought the shared memory file names of payload and event are even longer. But if this was fixed already some time ago, then your code adaptions should solve the name length issue good enough.

Copy link
Contributor

@hannemn hannemn left a comment

Choose a reason for hiding this comment

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

LGTM

@FlorianReimold FlorianReimold merged commit 4b14b74 into master Aug 2, 2023
12 checks passed
FlorianReimold pushed a commit that referenced this pull request Aug 2, 2023
FlorianReimold pushed a commit that referenced this pull request Aug 2, 2023
@KerstinKeller KerstinKeller deleted the feature/shm-monitoring-domain-default branch August 17, 2023 13:23
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.

3 participants