Because if there are mutiple access time in agent_state for the same
object, we should use the recently one.
2014-10-28 9:42 GMT+08:00 池信泽 <xmdxcxz@xxxxxxxxx>:
> I think there is also bug in ReplicatedPG::agent_estimate_atime_temp
> I think we should change the following code:
> for (map<time_t,HitSetRef>::iterator p = agent_state->hit_set_map.begin();
> p != agent_state->hit_set_map.end();
> ++p) {
> if (p->second->contains(oid)) {
> if (*atime < 0)
> *atime = now - p->first;
> if (temp)
> ++(*temp);
> else
> return;
> }
> }
>
> to something like this:
>
> 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 9:38 GMT+08:00 池信泽 <xmdxcxz@xxxxxxxxx>:
>> I think there is also bug in ReplicatedPG::agent_estimate_atime_temp
>> I think we should change the following code:
>> for (map<time_t,HitSetRef>::iterator p = agent_state->hit_set_map.begin();
>> p != agent_state->hit_set_map.end();
>> ++p) {
>> if (p->second->contains(oid)) {
>> if (*atime < 0)
>> *atime = now - p->first;
>> if (temp)
>> ++(*temp);
>> else
>> return;
>> }
>> }
>>
>> to something like this:
>>
>> 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 9:35 GMT+08:00 池信泽 <xmdxcxz@xxxxxxxxx>:
>>>
>>>
>>>
>>> 2014-10-28 9:24 GMT+08:00 池信泽 <xmdxcxz@xxxxxxxxx>:
>>>>
>>>> I think there is also bug in ReplicatedPG::agent_estimate_atime_temp
>>>> I think we should change the following code:
>>>> for (map<time_t,HitSetRef>::iterator p =
>>>> agent_state->hit_set_map.begin();
>>>> p != agent_state->hit_set_map.end();
>>>> ++p) {
>>>> if (p->second->contains(oid)) {
>>>> if (*atime < 0)
>>>> *atime = now - p->first;
>>>> if (temp)
>>>> ++(*temp);
>>>> else
>>>> return;
>>>> }
>>>> }
>>>>
>>>> to something like this:
>>>>
>>>> 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;
>>>> }
>>>> }
>>>>
>>>> Because if there are mutiple time_t in agent_state for the same object,
>>>> we should use the recently one.
>>>>
>>>> 2014-10-28 0:04 GMT+08:00 Sage Weil <sage@xxxxxxxxxxxx>:
>>>>>
>>>>> On Mon, 27 Oct 2014, Wang, Zhiqiang wrote:
>>>>> > Hi,
>>>>> >
>>>>> > I agree with you this is a bug.
>>>>> >
>>>>> > I think we should change the following code:
>>>>> >
>>>>> > // KISS: if [lower,upper] spans our target effort, evict it.
>>>>> > if (atime_lower >= agent_state->evict_effort)
>>>>> > return false;
>>>>> >
>>>>> > to something like this:
>>>>> >
>>>>> > if (1000000 - atime_upper >= agent_state->evict_effort)
>>>>> > return false;
>>>>> >
>>>>> > cc the ceph-devel list for comments.
>>>>>
>>>>> Yes, that does look backwards. Good catch! If you submit a pull request
>>>>> we can get it merged and then backported.
>>>>>
>>>>> Thanks!
>>>>> sage
>>>>>
>>>>>
>>>>> > ________________________________________
>>>>> > Date: Mon, 27 Oct 2014 10:20:01 +0800
>>>>> > Subject: Cache pool bug?
>>>>> > From: xmdxcxz@xxxxxxxxx
>>>>> > To: wonzhq@xxxxxxxxxxx
>>>>> > hi, zhiqiang?
>>>>> >
>>>>> > When I read the ReplicatedPG::agent_maybe_evic, I found than the
>>>>> > newest object has
>>>>> >
>>>>> > more probability to be evicted. For Example:
>>>>> >
>>>>> > the object in hit_set is newest object, call
>>>>> >
>>>>> > the funtion agent_estimate_atime_temp(soid, &atime, NULL /*FIXME
>>>>> > &temp*/); atime is 0 when return.
>>>>> >
>>>>> > agent_state->atime_hist.get_position_micro(atime, &atime_lower,
>>>>> > &atime_upper), the atime_lower is 0 when return.
>>>>> >
>>>>> > // KISS: if [lower,upper] spans our target effort, evict it.
>>>>> > if (atime_lower >= agent_state->evict_effort)
>>>>> > return false;
>>>>> >
>>>>> > Above, the newest object in hit_set is evicted.
>>>>> >
>>>>> > Is there a bug? Or, whether I skip something?
>>>>> >
>>>>> >
>>>>> > Thanks
>>>>> > N?????r??y??????X???v???)?{.n?????z?]z????ay? ????j ??f???h????? ?w???
>>>>> > ???j:+v???w???????? ????zZ+???????j"????i
>>>>
>>>>
>>>
>>
--
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
