Re: [PATCH] Fixed and polished SMP store stats collection

On 10/12/2011 05:00 PM, Amos Jeffries wrote:
> On Wed, 12 Oct 2011 15:27:23 -0600, Alex Rousskov wrote:
>> SMP shared memory cache stats were not collected.
>>
>> Mean disk object size stats were aggregated inaccurately for SMP.
>>
>> Moved Store-related stats into a dedicated StoreStats class,
>> encapsulating memory cache-related (mem), disk cache-related (swap), and
>> global store (number of objects) stats. Used consistent naming scheme to
>> make memory and disk stats more alike (we could create a class to
>> represent "store device" stats of sorts, but that seems like an overkill
>> at this time).
>
> Looks to me like the only difference between struct Mem and struct Swap
> is the "shared" variable member.
>
> You could cover that consistency problem and drop about 10 LOC by
> inheriting struct Mem from struct Swap if you don't want to combine them
> fully.

It did not feel right to inherit Mem from Swap so I merged common
properties in a parent "Store Part" class. I also moved open_disk_fd to
the swap part because those descriptors are counted only for
swap-related operations.

This did not reduce the LOC count, but it did remove code duplication,
and it would be easier to add more common swap/mem properties in the future.

> +1 on the current version anyway.

Committed to Squid3 trunk as r11794.

> Within reason we have free-reign over changes to cache mgr reports.
> The scripts I'm aware of all seem to scan for particular lines and
> parse their content if present.

Right, so the mgr:info changes would require changing those scripts
unless we add [duplicate] information without touching the old lines.

> Can you mock-up what the changed output would look like please?

Yes, that is the plan.

Thank you,

Alex.
Received on Fri Oct 14 2011 - 16:30:29 MDT