If it changed to be *atime = p->first
we should change ReplicatedPG::agent_maybe_evict
if (atime < 0 && obc->obs.oi.mtime != utime_t()) {
if (obc->obs.oi.local_mtime != utime_t()) {
atime = obc->obs.oi.local_mtime;
} else {
atime = obc->obs.oi.mtime;
}
}
if (atime < 0)
atime = 0; // or other
void ReplicatedPG::agent_estimate_atime_temp(const hobject_t& oid,
int *atime, int *temp)
{
assert(hit_set);
time_t now = ceph_clock_now(NULL).sec();
*atime = -1;
if (temp)
*temp = 0;
if (hit_set->contains(oid)) {
*atime = now;
if (temp)
++(*temp);
else
return;
}
for (map<time_t,HitSetRef>::iterator p = agent_state->hit_set_map.rbegin();
p != agent_state->hit_set_map.rend();
++p) {
if (p->second->contains(oid)) {
if (*atime < 0)
*atime = now - p->first;
if (temp)
++(*temp);
else
return;
}
}
}
2014-10-28 12:36 GMT+08:00 池信泽 <xmdxcxz@xxxxxxxxx>:
> I am sorry. We should only just modify the logic
> ReplicatedPG::agent_estimate_atime_temp, atime should be now.
>
> if (temp)
> *temp = 0;
> if (hit_set->contains(oid)) {
> *atime = 0;
> if (temp)
> ++(*temp);
> else
> return;
> }
>
> 2014-10-28 12:28 GMT+08:00 池信泽 <xmdxcxz@xxxxxxxxx>:
>> I think if it changed to be *atime = p->first, Below logic should
>> also be modified.
>>
>> ReplicatedPG::agent_maybe_evict
>>
>> if (atime < 0 && obc->obs.oi.mtime != utime_t()) {
>> if (obc->obs.oi.local_mtime != utime_t()) {
>> atime = ceph_clock_now(NULL).sec() - obc->obs.oi.local_mtime;
>> } else {
>> atime = ceph_clock_now(NULL).sec() - obc->obs.oi.mtime;
>> }
>> }
>>
>>
>> including
>> ReplicatedPG::agent_estimate_atime_temp
>> if (temp)
>> *temp = 0;
>> if (hit_set->contains(oid)) {
>> *atime = 0;
>> if (temp)
>> ++(*temp);
>> else
>> return;
>> }
>>
>> 2014-10-28 12:02 GMT+08:00 Sage Weil <sage@xxxxxxxxxxxx>:
>>> Yep. The pull request looks is correct too, but needs to be squashed into
>>> a single commit with a Signed-off-by: line and description.
>>>
>>> Zhiqiang, I think the reversed logic you just fixed in agent_maybe_evict()
>>> was partly because this function is filling in atime (which sounds like a
>>> timestamp) with an *age* (now - p->first). I think we should change
>>>
>>>> if (*atime < 0)
>>>> *atime = now - p->first;
>>>
>>> to be
>>>
>>> *atime = p->first
>>>
>>> and/or adjust the caller accordingly...?
>>>
>>> sage
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
