RE: Cache pool bug?

["Wang, Zhiqiang" <zhiqiang.wang@xxxxxxxxx>]

Good catch :) Looks correct to me.

From: 池信泽 [mailto:xmdxcxz@xxxxxxxxx] 
Sent: Tuesday, October 28, 2014 9:24 AM
To: Sage Weil
Cc: Wang, Zhiqiang; ceph-devel@xxxxxxxxxxxxxxx; wonzhq@xxxxxxxxxxx
Subject: Re: Cache pool bug?

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

��.n��������+%������w��{.n����z��u���ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f