pt-query-digest : Redundant argument in sprintf

Bug #1536305 reported by Romain GUINOT on 2016-01-20
10
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Percona Toolkit moved to https://jira.percona.com/projects/PT
Fix Released
High
Frank Cizmich

Bug Description

The current version of Perl (5.22.1) on Fedora has apparently stricter checks for arguments provided to sprintf.

Using the default report format, and using percona-toolkit 2.2.16 or previous versions, output of : pt-query-digest test.log returns :

Pipeline process 5 (iteration) caused an error: Redundant argument in sprintf at /opt/percona/toolkit/2.2.16/bin/pt-query-digest line 2556.
Will retry pipeline process 4 (iteration) 2 more times.

It is likely something introduced in recent versions of PerL.
Changing the return statement in sub shorten to :

   my $sprintf_expression = ($num =~ m/\./ || $n
         ? "%.${p}f%s"
         : '%d');

   return $units[$n] ? sprintf($sprintf_expression, $num, $units[$n]) : sprintf($sprintf_expression, $num);

makes the tool go a bit further without complaining but there are other sprintf statements to change as well.

Passing --report-format=profile returns without errors, so only specific / detailed modes are affected.
Let me know if you need more information.

Frederic Descamps (lefred) wrote :

similar issue than related in this bug: https://bugs.launchpad.net/percona-toolkit/+bug/1480719

Changed in percona-toolkit:
assignee: nobody → Frank Cizmich (frank-cizmich)
Romain GUINOT (romainguinot) wrote :

adding the partial patch mentioned in the original comment, in patch format.

Romain GUINOT (romainguinot) wrote :

adding an output sample exhibiting further sprintf problems, even after the partial patch has been applied.

Frank Cizmich (frank-cizmich) wrote :

Hi Romain

Ugh! yes. Will have to weed out the offending printf's one by one.

Thanks for the partial fix.

Meanwhile, a (lazy) workaround is to use

no warnings 'redundant';

in the affected packages within the file.

Changed in percona-toolkit:
status: New → Triaged
importance: Undecided → High
Frank Cizmich (frank-cizmich) wrote :
Changed in percona-toolkit:
status: Triaged → In Progress
milestone: none → 2.2.17
tags: added: pt-query-digest
Romain GUINOT (romainguinot) wrote :

Hi Frank,

I've just tried and yes the temporary workaround you mentioned works!
Much appreciated.

I've just needed to add it below the QueryReportFormatter package formatter

Romain GUINOT (romainguinot) wrote :

meant QueryReportFormatter package declaration

Frank Cizmich (frank-cizmich) wrote :

great!

thanks for the feedback.

Changed in percona-toolkit:
status: In Progress → Fix Committed
Romain GUINOT (romainguinot) wrote :

Hi Frank,

just to clarify, the "no warnings" i've added was on top of my previous sprintf partial fix.
Did you take this into account ?

Also, where is the percona toolkit code now ? launchpad says you've committed a fix but couldn't find a corresponding commit on LP or GitHub.

Thanks!
Romain.

Frank Cizmich (frank-cizmich) wrote :

Hi Romain,

Yes, I had that into account.
The sprintf issue appeared in more than one place.
The "unified" commit for that is here:
https://github.com/percona/percona-toolkit/pull/73/

Since the printf formats and values are dynamically assembled (by design), I opted for using indexes in the format symbols, which keeps Perl happy, although at the cost of readability. I admit I was on the balance on whether to use "no warnings 'redundant'" as a simpler "definite" solution, but that would theoretically break if the wording of the warning changes. :-/

If you encounter problems please report. Feedback is crucial.

Regards and again thanks Romain

Changed in percona-toolkit:
status: Fix Committed → Fix Released

Percona now uses JIRA for bug reports so this bug report is migrated to: https://jira.percona.com/browse/PT-395

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers