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

[Xrd] Wait for the current PollE poll to finish after removing a Link #1941

Merged
merged 1 commit into from
Mar 8, 2023

Conversation

smithdh
Copy link
Contributor

@smithdh smithdh commented Mar 6, 2023

This is a change to avoid issue #1928.

@amadio amadio requested a review from abh3 March 6, 2023 16:25
Copy link
Member

@abh3 abh3 left a comment

Choose a reason for hiding this comment

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

Just comment changes are needed. Presumably the code was tested with the "sleeps" that caused it to fail and now it doesn't fail, right? The only other suggestion is consider repackaging the "eventfd' processing into a separate class so that it can be used elsewhere should the need arise.

src/Xrd/XrdPollE.icc Outdated Show resolved Hide resolved
src/Xrd/XrdPollE.icc Outdated Show resolved Hide resolved
src/Xrd/XrdPollE.icc Outdated Show resolved Hide resolved
@smithdh
Copy link
Contributor Author

smithdh commented Mar 7, 2023

Thanks very much for the comments. I've changed those; the banners and renamed Wait4Poller. I've tried it with my "sleeps" it solves what I could provoke. (I don't know how to reproduce the original production-system problem, as that seems to be quite rare).

Repackaging as a separate class is likely a better way to make it. However I'm not sure I can find a nice general interface for it. (It would probably be used with an epoll-poller, as eventfd is linux specific like epoll). I don't think we have the same problem in XrdSysIOEventsPollE because of how that poller works. So I propose to leave it here (not separated into another class) now.

@abh3 abh3 merged commit 78128f9 into xrootd:master Mar 8, 2023
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.

None yet

2 participants