[sos-devel] addCopySpecLimit

[Jesse Jaggars]

On Tue, Mar 13, 2012 at 04:20:32PM +0000, Bryn M. Reeves wrote:
> On 03/13/2012 04:10 PM, Jesse Jaggars wrote:
> > There is a plugin method called addCopySpecLimit with the following
> > signature:
> > 
> > def addCopySpecLimit(self, fname, sizelimit=None, sub=None)
> > 
> > It's a fairly complicated function, as far as I can tell it does about
> > three things:
> 
> Right; it's not such a bad interface but the implementation is lacking
> at the moment. The intent is clear: copy files matching the copySpec up
> to the limit but as you noticed things aren't quite written that way today.
> 
> > 1. If fname is a single file it will attempt to copy the file.
> >   - if the file is under sizelimit it adds it via addCopySpec
> >   - if the file is over sizelimit it tails the last sizelimit bytes and
> >     writes that as an extCommand (via shelling out to tail)
> > 
> > 2. If fname is a glob it will attempt to copy (via addCopySpec) all the
> > files, stopping before it gets to sizelimit bytes total.
> > 
> > 3. If fname is a glob, but the first file it tries to copy is larger
> > than sizelimit bytes then nothing is added via addCopySpec.
> > 
> > There may be some problems with the above (especially #3) so I wanted to
> > start a thread on what the expectations for this function are and what
> > folks think this thing ought to do.
> 
> I think it's also worth considering whether addCopySpecLimit() is the
> best way to approach this. It complicates things even more but we would
> get more robust behaviour if we kept track of how much data we've
> accumulated and checked against a limit (either global or per copySpec).
> 

This is a good point, and a tough thing to attempt to tackle. We could
run into issues where we don't know what is most important (like you
talked about with sorting), but then a global option would be
configurable and optional. In the worst case we could ask the person
generating the report to send along what we lost (at the cost of asking
them to do something else.)

> > Here are my ideas:
> > 
> > * control the sorting
> > 
> > Currently files found via glob.glob are sorted alphabetically, which is
> > probably not what you want in some cases. It would be nice to be able to
> > sort the files however you wanted, or at least some parameter of
> > os.stat.
> 
> There are some complicated-ish rules to think about if we do something
> like this. For e.g. determining what is the "best" set of data to
> capture if we can't get everything. For log files it's not too hard -
> always get the latest (and they are the only current user of limits).

I really mean to propose allowing the plugin author to sort the files
how they like. They [should] know best which files are most important. A
simple implementation of this would be to allow the caller to pass in a
key function to be passed directly to sort(). This, of course, grows the
function signature on new optional parameter.

> 
> > * trim the first file in a glob if it is too big
> > 
> > Rather than copying nothing, it might be better to try to copy the first
> > file (post sort) according to the 'only one file passed in rule'.
> 
> The current code should attempt this:
> 
>         # Truncate the first file (others would likely be
>         # compressed),
>         # ensuring we get at least some logs
>         if flog == files[0] and limit_reached:
>             self.collectExtOutput("tail -c%d %s" % (sizelimit, flog),
>                 "tail_" + os.path.basename(flog))
> 
> It works OK in my testing.

Yes, you are right. If the first file is too big the function will
truncate it. Ignore my earlier complaint.

> 
> Regards,
> Bryn.

Jesse
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/sos-devel/attachments/20120313/6d6af5f4/attachment.sig>