-
Notifications
You must be signed in to change notification settings - Fork 149
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
"xrdcp -d 1" prints "XrdClient counters" only to STDOUT, not to other filedescriptors? #37
Labels
Comments
ghost
assigned ljanyst
Aug 29, 2013
The old client only gets critical bug fixes. |
Closed
amadio
pushed a commit
to amadio/xrootd
that referenced
this issue
May 17, 2023
Try to avoid usage of libradosstriper for readv operations since it may impact performance significantly. To do so we explicitly determine the objects that constitute a file and read from them using rados only. Reads are async. To do these async reads conveniently we introduce a class for handling multiple async read requests. * Initial implementation of ReadV at the XrdOss level * Correct the signature of ReadV to XrdCephOssFile * feat: do not use libradosstiper for readv operation * feat: use atomic operations for readv requests This should be the most efficient way of handling multiple read ops. * feat: use nonstriper reads for pread requests * feat: use nonstriper reads for read operations also To do so we do complete refactoring: bulkAioRead class moved to a separate file, and its features extended. Namely, it can do reads from files, not only objects, now. * feat: print warning message if waiting for aio reads from ceph takes long This is useful for debugging the reasons of failures for read(v) requests. * Added some comments * fix: use size_t for start_block We can use "%zx" in sprintf, so let's unify the types of variables in the function. This will also allow us to extend limitations on the file size. * feat: refactor BulkAioRead::read method, suggested during review 1. Rename end_block to last_block 2. Move variable definitions closer to its usage 3. Use 'std::min' instead of 'if' for chunk_len determination 4. Use more efficient chunk_start calculation * feat: add options to allow one to switch to standard read mechanisms This may be useful for testing. * feat: rename block_size to object_size in BulkAioRead New name better describes reality, since we are talking about the size of ceph objects. * feat: rename wait_for_complete to submit_and_wait_for_complete New name describes this function better. * feat: use more meaningful names for variables that loops over operations map op_data should describe the contents of the variables better. * feat: move type definitions into the class * feat: added comments with method's description * feat: remove unnecessary semicolons * feat: convert wait_for_complete method from void to int This allow one to improve several things. Here we change key to the operations and use object number instead of full its name. * fix: fixed comment * fix: fixed comments * feat: refactor bulkAioRead class Pointers were dropped from objectReadOperation and ceph_bufferlist objects. The objects are moved to appropriate classes to simplify memory management and usage. * feat: take into account completion's return value We can retrieve return code from completion and get meaningful status of the whole operation with this value. * feat: allow reading of sparse file Since we do not really expect sparse files, we use a fallback mechanism: if a read(v) failed with -ENOENT exit status, then just resubmit it using striper-based functions. * lint: remove trailing whitespaces * feat: use meaningful names for read(v) functions The name now indicates whether read(v)s are striper or non-striper ones. * feat: fallback to striper-based read if number of stripes > 1 Just in case, such files should not be present in our production setup * feat: allow zero-sized reads In principle, this is a correct request, so we should support it. * fix: make sure we do not delete completion objects until submitted operation is completed This is done to prevent some nasty side-effects, e.g. writing to a deleted buffer. * fix: remove move constructor from bulkAioRead We do not use it. * fix: handle failure to allocate completion Completion allocation can fail, we should take that into an account. * feat: use file reference to construct readOp objects There is no need to extract (and the copy) file name and object size from file reference to construct read object, we can use file reference directly. * feat: replace conversion operator with explicit method Implicit conversion was making code less readable. * feat: remove call to is_complete() in completion wrapper destructor There is no need to check for completion, we can call wait_for_complete multiple times. * feat: put warning threshold to config file It is better to have this value as configurable instead of hardcoded. * fix: initialize return code variable in ReadOpData * Added comment * feat: add comment for future optimization. We should use `aio_cancel` to cancel all pending read operations in future. * fix: remove vim's swp file Commited by accident * feat: improve logging Add file descriptor to sparse file's logging, fix typos. * fix: minor fixes Remove unnecessary include, move variable declaration closer to the usage, fix spelling in the comment. * feat: BulkAioRead::read method refactoring Refactoring was made to increase (hopefully) readability. * fix: better wording for comment * feat: BulkAioRead::read -- change loop exit condition We can exit when `to_read == 0`. This allow us to drop `end_block` variable. * fix: add call to `clear` after getting results This is to allow clients to use the same readOp object for future operations. --------- Co-authored-by: Ian Johnson <ijjorama@gmail.com> Co-authored-by: Alexander Rogovskiy <alexander.rogovskiy@stfc.ac.uk>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Apparently the "XrdClient counters:" cannot be redirected to file - "xrdcp" does not emit them if STDOUT is redirected. Which is odd, since the preceding "Low level caching info:" is printed. Suggest to allow redirecting, these might have useful info and the overall amount of data compared to e.g. STDERR is negligible.
$ xrdcp -d 1 -f root://eospps//eos/testfile /dev/null 2>/dev/null
Low level caching info:
StallsRate=1
StallsCount=1
ReadsCounter=1
BytesUsefulness=0
BytesSubmitted=2097152 BytesHit=0
XrdClient counters:
ReadBytes: 1048576
WrittenBytes: 0
WriteRequests: 0
ReadRequests: 1
ReadMisses: 1
ReadHits: 0
ReadMissRate: 1.000000
ReadVRequests: 0
ReadVSubRequests: 0
ReadVSubChunks: 0
ReadVBytes: 0
ReadVAsyncRequests: 0
ReadVAsyncSubRequests: 0
ReadVAsyncSubChunks: 0
ReadVAsyncBytes: 0
ReadAsyncRequests: 0
ReadAsyncBytes: 0
$ xrdcp -d 1 -f root://eospps//eos/testfile /dev/null 2>/dev/null >/tmp/sfdhjfd; cat /tmp/sfdhjfd
Low level caching info:
StallsRate=1
StallsCount=1
ReadsCounter=1
BytesUsefulness=0
BytesSubmitted=2097152 BytesHit=0
$ rpm -qf
which xrdcp
xrootd-client-3.3.3-1.slc6.x86_64
The text was updated successfully, but these errors were encountered: