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

Throttle implementation #165

Merged
merged 24 commits into from
Feb 1, 2015
Merged

Throttle implementation #165

merged 24 commits into from
Feb 1, 2015

Conversation

bbockelm
Copy link
Contributor

Latest attempt at merging the throttles code. Brushed off the spiderwebs, fixed some atomics issues; everything looks good right now.

Works fine with an OSS; throttles don't apply for AIO or SendFile. Not sure what I'd need to do for either of those.

Conflicts:
	src/XrdSys/XrdSysAtomics.hh
Conflicts:
	packaging/rhel/xrootd.spec.in
	src/XrdPlugins.cmake
	src/XrdSys/XrdSysAtomics.hh
@xrootd-dev
Copy link

Hi Brian,

You can make throttle apply to sendfile if you enable senddata. So, I
guess I will have to look to see what I should change there -- unless you
want to do it.

Andy

On Sun, 16 Nov 2014, Brian Bockelman wrote:

Latest attempt at merging the throttles code. Brushed off the spiderwebs, fixed some atomics issues; everything looks good right now.

Works fine with an OSS; throttles don't apply for AIO or SendFile. Not sure what I'd need to do for either of those.
You can merge this Pull Request by running:

git pull https://github.com/bbockelm/xrootd throttles-nov-2014

Or you can view, comment on it, or merge it online at:

#165

-- Commit Summary --

  • First version of the throttle manager.
  • Begin to integrate throttle manager into Xrootd daemon.
  • Hook throttling mechanism into server buffer management.
  • Handle throttles correctly for sendfile/mmap case. Hook throttle IDs into file UID.
  • Debug AIO and normal IO throttling.
  • Separate throttle manager from buffer manager. Implement username hash algorithm.
  • Fix parsing of the IOPS configuration string.
  • Implement ability to track IO load. Implement ability to shed load when throttles are hit.
  • Fix IO load calculation bugs.
  • Log loudly when we load-shed a client.
  • Fix Linux compilation issues.
  • Revert change to rpCheck const'ness.
  • Creation of the basic throttle SFS plugin classes.
  • Further remove old implementation from Xrd and XrdXrootd. Finish implementation of XrdThrottle SFS.
  • Finish implementation of throttling plugin.
  • Add XrdThrottle to packaging.
  • Merge branch 'master' into throttles_v2
  • Add link to XrdOfs directly to prevent complaints from Mac OS X linker.
  • Remove unnecessary linking of throttle manager.
  • Merge branch 'master' into throttles_v2
  • Fix small compilation issues from new atomics and warnings.
  • Use SFS from Xrootd, if available.

-- File Changes --

M packaging/rhel/xrootd.spec.in (1)
M src/XrdPlugins.cmake (28)
A src/XrdThrottle/XrdThrottle.hh (227)
A src/XrdThrottle/XrdThrottleFile.cc (158)
A src/XrdThrottle/XrdThrottleFileSystem.cc (151)
A src/XrdThrottle/XrdThrottleFileSystemConfig.cc (351)
A src/XrdThrottle/XrdThrottleManager.cc (375)
A src/XrdThrottle/XrdThrottleManager.hh (199)
A src/XrdThrottle/XrdThrottleTrace.hh (42)

-- Patch Links --

https://github.com/xrootd/xrootd/pull/165.patch
https://github.com/xrootd/xrootd/pull/165.diff


Reply to this email directly or view it on GitHub:
#165

########################################################################
Use REPLY-ALL to reply to list

To unsubscribe from the XROOTD-DEV list, click the following link:
https://listserv.slac.stanford.edu/cgi-bin/wa?SUBED1=XROOTD-DEV&A=1

@bbockelm
Copy link
Contributor Author

Hi Andy,

If possible, I'd prefer to work on getting this code reviewed / committed, then enable the missing use cases.

Brian

@abh3
Copy link
Member

abh3 commented Nov 18, 2014

Hi Brian,

Would you mind if we delay merging this in until after we cut 4.1? That release is already complicated enough and I'd like the least amount of noise right before it's supposed to come out.

Andy

@bbockelm
Copy link
Contributor Author

Sure, no problem. That'll give me time to look at SendData then!

@bbockelm
Copy link
Contributor Author

Hm - SendData implementation was far easier than originally feared. Just pushed a new commit.

In general, reviewing the code again:

  • mmap-based reads are disabled when throttles are loaded.
  • Old-style sendfile-based reads are disabled. New-style, SendData-based reads should work and be throttled correctly.
  • Asynchronous IO requests are converted to synchronous IO in order to be throttled.

So - unless I missed something - when the throttle is loaded, all reads mechanisms are properly accounted for or disabled.

For reference, in order to enable this plugin, you need to do:
xrootd.fslib throttle default
throttle.throttle concurrency 10 data 10485760
throttle.trace all

Line 1 enables the throttle; line 2 sets the throttles to 10 outstanding IO requests and 10MB/s of data; line 3 enables all possible debugging.

@abh3
Copy link
Member

abh3 commented Nov 18, 2014

Thanks! Could you point me to the full documentation and all the various options and what they really do? It would be nice to get this documented in the standard format. As for the three lines, my assumption is that the "trace all" is not necessary regardless of the implication that it is :-)

@bbockelm
Copy link
Contributor Author

Ah, yes - I should have written "an example is..."

What's the best way to include documentation? Should I write it on the ticket or elsewhere?

@abh3
Copy link
Member

abh3 commented Nov 18, 2014

It doesn't much matter. A README in the source directory will do for now. I use that to enter it into a word document.

@bbockelm
Copy link
Contributor Author

Alright, documentation updated!

abh3 added a commit that referenced this pull request Feb 1, 2015
@abh3 abh3 merged commit d395913 into xrootd:master Feb 1, 2015
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

3 participants