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