---------- Forwarded message ----------
From: 池信泽 <xmdxcxz@xxxxxxxxx>
Date: 2014-10-28 13:37 GMT+08:00
Subject: Re: Cache pool bug?
To: "Wang, Zhiqiang" <zhiqiang.wang@xxxxxxxxx>
If we use the timestamp,
get_position_micro(int32_t v, uint64_t *lower, uint64_t *upper)
unsigned bin = calc_bits_of(v); // this function will return the
same value at most time, this may be a problem.
2014-10-28 12:50 GMT+08:00 Wang, Zhiqiang <zhiqiang.wang@xxxxxxxxx>:
> Yes, if we change atime to be a timestamp, we also need to change the code for the case when the object is not in any hit sets.
>
> -----Original Message-----
> From: 池信泽 [mailto:xmdxcxz@xxxxxxxxx]
> Sent: Tuesday, October 28, 2014 12:46 PM
> To: Sage Weil; Wang, Zhiqiang; ceph-devel@xxxxxxxxxxxxxxx
> Subject: Re: Cache pool bug?
>
> 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
