diff mbox

About the "temperature histogram" of tier-agent

Message ID CAHMqkF7XOhzrQJODwOGnz-0Y8echtdQPEamcNL126w5ctW4+Xg@mail.gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

YuFan Chen Nov. 28, 2017, 6:38 a.m. UTC 
Hi,

   Before Jewel, the code uses "atime" to distinguish hot/cold data:

          agent_estimate_atime_temp() calculates the distance.
          It means colder data gets longer distance (larger value)


   In Jewel, it changes to use temperature with weight.
            So, colder data gets smaller value.

    However, it still use the "old semantics" to determine evicting or not in
           ReplicatedPG::agent_estimate_atime_temp();

    if (1000000 - temp_upper >= agent_state->evict_effort)
       return false;


     if there is no misunderstanding, should it change to that:

   }

Yu Fan
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Sage Weil Nov. 28, 2017, 1:20 p.m. UTC | #1 
Hi MingXin, Li,

Does the below make sense to you?

Thanks!
sage


On Tue, 28 Nov 2017, YuFan Chen wrote:

> Hi,
> 
>    Before Jewel, the code uses "atime" to distinguish hot/cold data:
> 
>           agent_estimate_atime_temp() calculates the distance.
>           It means colder data gets longer distance (larger value)
> 
> 
>    In Jewel, it changes to use temperature with weight.
>             So, colder data gets smaller value.
> 
>     However, it still use the "old semantics" to determine evicting or not in
>            ReplicatedPG::agent_estimate_atime_temp();
> 
>     if (1000000 - temp_upper >= agent_state->evict_effort)
>        return false;
> 
> 
>      if there is no misunderstanding, should it change to that:
> 
> --- a/src/osd/ReplicatedPG.cc
> +++ b/src/osd/ReplicatedPG.cc
> @@ -12211,7 +12211,7 @@ bool
> ReplicatedPG::agent_maybe_evict(ObjectContextRef& obc, bool
> after_flush)
>      delete f;
>      *_dout << dendl;
> 
> -    if (1000000 - temp_upper >= agent_state->evict_effort)
> +    if (temp_upper >= agent_state->evict_effort)
>        return false;
>    }
> 
> Yu Fan
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Li Wang Nov. 30, 2017, 8:51 a.m. UTC | #2 
Hi,
  This makes sense, the variable name 'temp_upper' is really confusing
which in fact does not count the number of hot objects. However, personally,
I prefer use  temp_lower as follows,

if (temp_lower >= agent_state->evict_effort)
  return false;

Cheers,
Li Wang

2017-11-28 21:20 GMT+08:00 Sage Weil <sage@newdream.net>:
> Hi MingXin, Li,
>
> Does the below make sense to you?
>
> Thanks!
> sage
>
>
> On Tue, 28 Nov 2017, YuFan Chen wrote:
>
>> Hi,
>>
>>    Before Jewel, the code uses "atime" to distinguish hot/cold data:
>>
>>           agent_estimate_atime_temp() calculates the distance.
>>           It means colder data gets longer distance (larger value)
>>
>>
>>    In Jewel, it changes to use temperature with weight.
>>             So, colder data gets smaller value.
>>
>>     However, it still use the "old semantics" to determine evicting or not in
>>            ReplicatedPG::agent_estimate_atime_temp();
>>
>>     if (1000000 - temp_upper >= agent_state->evict_effort)
>>        return false;
>>
>>
>>      if there is no misunderstanding, should it change to that:
>>
>> --- a/src/osd/ReplicatedPG.cc
>> +++ b/src/osd/ReplicatedPG.cc
>> @@ -12211,7 +12211,7 @@ bool
>> ReplicatedPG::agent_maybe_evict(ObjectContextRef& obc, bool
>> after_flush)
>>      delete f;
>>      *_dout << dendl;
>>
>> -    if (1000000 - temp_upper >= agent_state->evict_effort)
>> +    if (temp_upper >= agent_state->evict_effort)
>>        return false;
>>    }
>>
>> Yu Fan
>> --
>> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Li Wang Dec. 1, 2017, 12:56 p.m. UTC | #3 
Hi,
  Would you please send a pr to fix this

Cheers,
Li Wang

2017-11-30 23:45 GMT+08:00 YuFan Chen <wiz.chen@gmail.com>:
> Hi,
>   Yes, it really means the ranking level in a
>    'temperature' rating scale.
>   Using 'temp_lower' would be better.
>
> Thanks.
>
> Yufan Chen
>
>
> Li Wang <laurence.liwang@gmail.com> 於 2017年11月30日 週四 下午4:51 寫道：
>>
>> Hi,
>>   This makes sense, the variable name 'temp_upper' is really confusing
>> which in fact does not count the number of hot objects. However,
>> personally,
>> I prefer use  temp_lower as follows,
>>
>> if (temp_lower >= agent_state->evict_effort)
>>   return false;
>>
>> Cheers,
>> Li Wang
>>
>> 2017-11-28 21:20 GMT+08:00 Sage Weil <sage@newdream.net>:
>> > Hi MingXin, Li,
>> >
>> > Does the below make sense to you?
>> >
>> > Thanks!
>> > sage
>> >
>> >
>> > On Tue, 28 Nov 2017, YuFan Chen wrote:
>> >
>> >> Hi,
>> >>
>> >>    Before Jewel, the code uses "atime" to distinguish hot/cold data:
>> >>
>> >>           agent_estimate_atime_temp() calculates the distance.
>> >>           It means colder data gets longer distance (larger value)
>> >>
>> >>
>> >>    In Jewel, it changes to use temperature with weight.
>> >>             So, colder data gets smaller value.
>> >>
>> >>     However, it still use the "old semantics" to determine evicting or
>> >> not in
>> >>            ReplicatedPG::agent_estimate_atime_temp();
>> >>
>> >>     if (1000000 - temp_upper >= agent_state->evict_effort)
>> >>        return false;
>> >>
>> >>
>> >>      if there is no misunderstanding, should it change to that:
>> >>
>> >> --- a/src/osd/ReplicatedPG.cc
>> >> +++ b/src/osd/ReplicatedPG.cc
>> >> @@ -12211,7 +12211,7 @@ bool
>> >> ReplicatedPG::agent_maybe_evict(ObjectContextRef& obc, bool
>> >> after_flush)
>> >>      delete f;
>> >>      *_dout << dendl;
>> >>
>> >> -    if (1000000 - temp_upper >= agent_state->evict_effort)
>> >> +    if (temp_upper >= agent_state->evict_effort)
>> >>        return false;
>> >>    }
>> >>
>> >> Yu Fan
>> >> --
>> >> To unsubscribe from this list: send the line "unsubscribe ceph-devel"
>> >> in
>> >> the body of a message to majordomo@vger.kernel.org
>> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> >>
>> >>
>> > --
>> > To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
>> > the body of a message to majordomo@vger.kernel.org
>> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
YuFan Chen Dec. 1, 2017, 3:12 p.m. UTC | #4 
Okay. I will do it.

Yufan Chen

2017-12-01 20:56 GMT+08:00 Li Wang <laurence.liwang@gmail.com>:
> Hi,
>   Would you please send a pr to fix this
>
> Cheers,
> Li Wang
>
> 2017-11-30 23:45 GMT+08:00 YuFan Chen <wiz.chen@gmail.com>:
>> Hi,
>>   Yes, it really means the ranking level in a
>>    'temperature' rating scale.
>>   Using 'temp_lower' would be better.
>>
>> Thanks.
>>
>> Yufan Chen
>>
>>
>> Li Wang <laurence.liwang@gmail.com> 於 2017年11月30日 週四 下午4:51 寫道：
>>>
>>> Hi,
>>>   This makes sense, the variable name 'temp_upper' is really confusing
>>> which in fact does not count the number of hot objects. However,
>>> personally,
>>> I prefer use  temp_lower as follows,
>>>
>>> if (temp_lower >= agent_state->evict_effort)
>>>   return false;
>>>
>>> Cheers,
>>> Li Wang
>>>
>>> 2017-11-28 21:20 GMT+08:00 Sage Weil <sage@newdream.net>:
>>> > Hi MingXin, Li,
>>> >
>>> > Does the below make sense to you?
>>> >
>>> > Thanks!
>>> > sage
>>> >
>>> >
>>> > On Tue, 28 Nov 2017, YuFan Chen wrote:
>>> >
>>> >> Hi,
>>> >>
>>> >>    Before Jewel, the code uses "atime" to distinguish hot/cold data:
>>> >>
>>> >>           agent_estimate_atime_temp() calculates the distance.
>>> >>           It means colder data gets longer distance (larger value)
>>> >>
>>> >>
>>> >>    In Jewel, it changes to use temperature with weight.
>>> >>             So, colder data gets smaller value.
>>> >>
>>> >>     However, it still use the "old semantics" to determine evicting or
>>> >> not in
>>> >>            ReplicatedPG::agent_estimate_atime_temp();
>>> >>
>>> >>     if (1000000 - temp_upper >= agent_state->evict_effort)
>>> >>        return false;
>>> >>
>>> >>
>>> >>      if there is no misunderstanding, should it change to that:
>>> >>
>>> >> --- a/src/osd/ReplicatedPG.cc
>>> >> +++ b/src/osd/ReplicatedPG.cc
>>> >> @@ -12211,7 +12211,7 @@ bool
>>> >> ReplicatedPG::agent_maybe_evict(ObjectContextRef& obc, bool
>>> >> after_flush)
>>> >>      delete f;
>>> >>      *_dout << dendl;
>>> >>
>>> >> -    if (1000000 - temp_upper >= agent_state->evict_effort)
>>> >> +    if (temp_upper >= agent_state->evict_effort)
>>> >>        return false;
>>> >>    }
>>> >>
>>> >> Yu Fan
>>> >> --
>>> >> To unsubscribe from this list: send the line "unsubscribe ceph-devel"
>>> >> in
>>> >> the body of a message to majordomo@vger.kernel.org
>>> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>> >>
>>> >>
>>> > --
>>> > To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
>>> > the body of a message to majordomo@vger.kernel.org
>>> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

--- a/src/osd/ReplicatedPG.cc
+++ b/src/osd/ReplicatedPG.cc
@@ -12211,7 +12211,7 @@  bool
ReplicatedPG::agent_maybe_evict(ObjectContextRef& obc, bool
after_flush)
     delete f;
     *_dout << dendl;

-    if (1000000 - temp_upper >= agent_state->evict_effort)
+    if (temp_upper >= agent_state->evict_effort)
       return false;