You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficserver.apache.org by jrushford <gi...@git.apache.org> on 2016/10/11 21:44:59 UTC

[GitHub] trafficserver pull request #1097: Make the use of madvise() with MADV_DONTDU...

GitHub user jrushford opened a pull request:

    https://github.com/apache/trafficserver/pull/1097

    Make the use of madvise() with MADV_DONTDUMP configurable.

    We have seen high cpu loads and high time to serve problems in our production platform when using madvise() with the MADV_DONTDUMP option.   This appears to be a kernel issue with madvise().  in order to avoid having to rebuild ats, this PR uses a patch from TS-3417 to make the use of madvise() with the MADV_DONTDUMP flag configurable.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/jrushford/trafficserver TS-4957

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/trafficserver/pull/1097.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1097
    
----
commit f1c37818622fc0fc4e05614c7f1f83a10fe8c063
Author: John J. Rushford <jr...@apache.org>
Date:   2016-10-11T21:37:01Z

    Make the use of madvise() with MADV_DONTDUMP configurable.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1097: Make the use of madvise() with MADV_DONTDU...

Posted by jrushford <gi...@git.apache.org>.
Github user jrushford commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1097#discussion_r82898723
  
    --- Diff: mgmt/RecordsConfig.cc ---
    @@ -1449,6 +1449,8 @@ static const RecordElement RecordsConfig[] =
       ,
       {RECT_CONFIG, "proxy.config.allocator.hugepages", RECD_INT, "0", RECU_RESTART_TS, RR_NULL, RECC_NULL, "[0-1]", RECA_NULL}
       ,
    +  {RECT_CONFIG, "proxy.config.allocator.dontdump_iobuffers", RECD_INT, "0", RECU_RESTART_TS, RR_NULL, RECC_NULL, "[0-1]", RECA_NULL}
    --- End diff --
    
    Okay, I'll make the change and update the doc.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver issue #1097: Make the use of madvise() with MADV_DONTDUMP conf...

Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:

    https://github.com/apache/trafficserver/pull/1097
  
    FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/1040/ for details.
     



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver issue #1097: Make the use of madvise() with MADV_DONTDUMP conf...

Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on the issue:

    https://github.com/apache/trafficserver/pull/1097
  
    No I think it is trivial to remove the global and we should just do it
    
    > On Oct 18, 2016, at 7:29 AM, John J. Rushford <no...@github.com> wrote:
    > 
    > @jrushford commented on this pull request.
    > 
    > In iocore/eventsystem/I_IOBuffer.h:
    > 
    > > @@ -58,6 +58,7 @@ class VIO;
    >  inkcoreapi extern int64_t max_iobuffer_size;
    >  extern int64_t default_small_iobuffer_size;
    >  extern int64_t default_large_iobuffer_size; // matched to size of OS buffers
    > +extern int iobuffer_advice;
    > @jpeach I'd like to land this as is and write a JIRA to come back to the global variable once we've had time to evaluate this. Are you okay with this?
    > 
    > \u2014
    > You are receiving this because you were mentioned.
    > Reply to this email directly, view it on GitHub, or mute the thread.
    > 



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver issue #1097: Make the use of madvise() with MADV_DONTDUMP conf...

Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:

    https://github.com/apache/trafficserver/pull/1097
  
    Linux build *failed*! See https://ci.trafficserver.apache.org/job/Github-Linux/936/ for details.
     



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver issue #1097: Make the use of madvise() with MADV_DONTDUMP conf...

Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:

    https://github.com/apache/trafficserver/pull/1097
  
    Linux build *successful*! See https://ci.trafficserver.apache.org/job/Github-Linux/866/ for details.
     



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1097: Make the use of madvise() with MADV_DONTDU...

Posted by jrushford <gi...@git.apache.org>.
Github user jrushford commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1097#discussion_r83866632
  
    --- Diff: iocore/eventsystem/I_IOBuffer.h ---
    @@ -58,6 +58,7 @@ class VIO;
     inkcoreapi extern int64_t max_iobuffer_size;
     extern int64_t default_small_iobuffer_size;
     extern int64_t default_large_iobuffer_size; // matched to size of OS buffers
    +extern int iobuffer_advice;
    --- End diff --
    
    @jpeach I'd like to land this as is and write a JIRA to come back to the global variable once we've had time to evaluate this.  Are you okay with this?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1097: Make the use of madvise() with MADV_DONTDU...

Posted by jrushford <gi...@git.apache.org>.
Github user jrushford commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1097#discussion_r82898472
  
    --- Diff: iocore/eventsystem/EventSystem.cc ---
    @@ -51,5 +51,15 @@ ink_event_system_init(ModuleVersion v)
       if (default_large_iobuffer_size > max_iobuffer_size) {
         default_large_iobuffer_size = max_iobuffer_size;
       }
    +
    +#ifdef MADV_DONTDUMP // This should only exist on Linux 3.4 and higher.
    +  bool dont_dump_enabled;
    +  RecGetRecordBool("proxy.config.allocator.dontdump_iobuffers", (RecBool *)&dont_dump_enabled, false);
    +
    +  if (dont_dump_enabled) {
    +    iobuffer_advice = MADV_DONTDUMP;
    --- End diff --
    
    Good idea, I'll make the change.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1097: Make the use of madvise() with MADV_DONTDU...

Posted by PSUdaemon <gi...@git.apache.org>.
Github user PSUdaemon commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1097#discussion_r83040861
  
    --- Diff: iocore/eventsystem/EventSystem.cc ---
    @@ -51,5 +51,15 @@ ink_event_system_init(ModuleVersion v)
       if (default_large_iobuffer_size > max_iobuffer_size) {
         default_large_iobuffer_size = max_iobuffer_size;
       }
    +
    +#ifdef MADV_DONTDUMP // This should only exist on Linux 3.4 and higher.
    +  bool dont_dump_enabled;
    --- End diff --
    
    If the default is true, shouldn't we initialize to true?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver issue #1097: Make the use of madvise() with MADV_DONTDUMP conf...

Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:

    https://github.com/apache/trafficserver/pull/1097
  
    FreeBSD build *failed*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/993/ for details.
     



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver issue #1097: Make the use of madvise() with MADV_DONTDUMP conf...

Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:

    https://github.com/apache/trafficserver/pull/1097
  
    Linux build *failed*! See https://ci.trafficserver.apache.org/job/Github-Linux/885/ for details.
     



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver issue #1097: TS-4957: Make the use of madvise() with MADV_DONT...

Posted by PSUdaemon <gi...@git.apache.org>.
Github user PSUdaemon commented on the issue:

    https://github.com/apache/trafficserver/pull/1097
  
    I don't care about the name as long as it's not uselessly confusing.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver issue #1097: Make the use of madvise() with MADV_DONTDUMP conf...

Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:

    https://github.com/apache/trafficserver/pull/1097
  
    FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/971/ for details.
     



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1097: Make the use of madvise() with MADV_DONTDU...

Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1097#discussion_r83048130
  
    --- Diff: iocore/eventsystem/EventSystem.cc ---
    @@ -51,5 +51,15 @@ ink_event_system_init(ModuleVersion v)
       if (default_large_iobuffer_size > max_iobuffer_size) {
         default_large_iobuffer_size = max_iobuffer_size;
       }
    +
    +#ifdef MADV_DONTDUMP // This should only exist on Linux 3.4 and higher.
    +  bool dont_dump_enabled;
    --- End diff --
    
    Yes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver issue #1097: Make the use of madvise() with MADV_DONTDUMP conf...

Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:

    https://github.com/apache/trafficserver/pull/1097
  
    Linux build *successful*! See https://ci.trafficserver.apache.org/job/Github-Linux/933/ for details.
     



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1097: Make the use of madvise() with MADV_DONTDU...

Posted by jrushford <gi...@git.apache.org>.
Github user jrushford commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1097#discussion_r83227421
  
    --- Diff: iocore/eventsystem/EventSystem.cc ---
    @@ -51,5 +51,15 @@ ink_event_system_init(ModuleVersion v)
       if (default_large_iobuffer_size > max_iobuffer_size) {
         default_large_iobuffer_size = max_iobuffer_size;
       }
    +
    +#ifdef MADV_DONTDUMP // This should only exist on Linux 3.4 and higher.
    +  bool dont_dump_enabled;
    --- End diff --
    
    done


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1097: Make the use of madvise() with MADV_DONTDU...

Posted by PSUdaemon <gi...@git.apache.org>.
Github user PSUdaemon commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1097#discussion_r82896517
  
    --- Diff: mgmt/RecordsConfig.cc ---
    @@ -1449,6 +1449,8 @@ static const RecordElement RecordsConfig[] =
       ,
       {RECT_CONFIG, "proxy.config.allocator.hugepages", RECD_INT, "0", RECU_RESTART_TS, RR_NULL, RECC_NULL, "[0-1]", RECA_NULL}
       ,
    +  {RECT_CONFIG, "proxy.config.allocator.dontdump_iobuffers", RECD_INT, "0", RECU_RESTART_TS, RR_NULL, RECC_NULL, "[0-1]", RECA_NULL}
    --- End diff --
    
    I think we should default this to 1


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver issue #1097: Make the use of madvise() with MADV_DONTDUMP conf...

Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:

    https://github.com/apache/trafficserver/pull/1097
  
    FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/975/ for details.
     



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver issue #1097: Make the use of madvise() with MADV_DONTDUMP conf...

Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:

    https://github.com/apache/trafficserver/pull/1097
  
    Linux build *successful*! See https://ci.trafficserver.apache.org/job/Github-Linux/863/ for details.
     



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1097: Make the use of madvise() with MADV_DONTDU...

Posted by PSUdaemon <gi...@git.apache.org>.
Github user PSUdaemon commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1097#discussion_r82918082
  
    --- Diff: mgmt/RecordsConfig.cc ---
    @@ -1449,6 +1449,8 @@ static const RecordElement RecordsConfig[] =
       ,
       {RECT_CONFIG, "proxy.config.allocator.hugepages", RECD_INT, "0", RECU_RESTART_TS, RR_NULL, RECC_NULL, "[0-1]", RECA_NULL}
       ,
    +  {RECT_CONFIG, "proxy.config.allocator.dontdump_iobuffers", RECD_INT, "0", RECU_RESTART_TS, RR_NULL, RECC_NULL, "[0-1]", RECA_NULL}
    --- End diff --
    
    I'm fine with it being backported if @bryancall is. Though, I don't think we absolutely have to. In 7.0.x it's on by default now, you just can't disable it.
    
    I'm considering backporting this to 6.2.x as well due to the trouble it causes on older kernels.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver issue #1097: TS-4957: Make the use of madvise() with MADV_DONT...

Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:

    https://github.com/apache/trafficserver/pull/1097
  
    FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/1056/ for details.
     



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver issue #1097: TS-4957: Make the use of madvise() with MADV_DONT...

Posted by jrushford <gi...@git.apache.org>.
Github user jrushford commented on the issue:

    https://github.com/apache/trafficserver/pull/1097
  
    Thanks @jpeach.  FWIW, this patch is originally from TS-3417 and @PSUdaemon and I dug it up after we discovered we had a problem with madvise() and to fix, required deploying a new rpm to 1300+ proxies.  I'm okay with the name change and will make it if @zwoop and @PSUdaemon agree.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver issue #1097: Make the use of madvise() with MADV_DONTDUMP conf...

Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:

    https://github.com/apache/trafficserver/pull/1097
  
    Linux build *successful*! See https://ci.trafficserver.apache.org/job/Github-Linux/886/ for details.
     



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver issue #1097: Make the use of madvise() with MADV_DONTDUMP conf...

Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:

    https://github.com/apache/trafficserver/pull/1097
  
    FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/1044/ for details.
     



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver issue #1097: Make the use of madvise() with MADV_DONTDUMP conf...

Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:

    https://github.com/apache/trafficserver/pull/1097
  
    FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/994/ for details.
     



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver issue #1097: Make the use of madvise() with MADV_DONTDUMP conf...

Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:

    https://github.com/apache/trafficserver/pull/1097
  
    FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/974/ for details.
     



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver issue #1097: Make the use of madvise() with MADV_DONTDUMP conf...

Posted by jrushford <gi...@git.apache.org>.
Github user jrushford commented on the issue:

    https://github.com/apache/trafficserver/pull/1097
  
    @jpeach and @PSUdaemon - I've made iobuffer_advice a local variable.  Tested and it is working okay.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver issue #1097: Make the use of madvise() with MADV_DONTDUMP conf...

Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:

    https://github.com/apache/trafficserver/pull/1097
  
    Linux build *successful*! See https://ci.trafficserver.apache.org/job/Github-Linux/938/ for details.
     



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1097: Make the use of madvise() with MADV_DONTDU...

Posted by jrushford <gi...@git.apache.org>.
Github user jrushford commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1097#discussion_r82902604
  
    --- Diff: iocore/eventsystem/EventSystem.cc ---
    @@ -51,5 +51,15 @@ ink_event_system_init(ModuleVersion v)
       if (default_large_iobuffer_size > max_iobuffer_size) {
         default_large_iobuffer_size = max_iobuffer_size;
       }
    +
    +#ifdef MADV_DONTDUMP // This should only exist on Linux 3.4 and higher.
    +  bool dont_dump_enabled;
    +  RecGetRecordBool("proxy.config.allocator.dontdump_iobuffers", (RecBool *)&dont_dump_enabled, false);
    +
    +  if (dont_dump_enabled) {
    +    iobuffer_advice = MADV_DONTDUMP;
    --- End diff --
    
    Changes were made and squashed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver issue #1097: TS-4957: Make the use of madvise() with MADV_DONT...

Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:

    https://github.com/apache/trafficserver/pull/1097
  
    Linux build *successful*! See https://ci.trafficserver.apache.org/job/Github-Linux/948/ for details.
     



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1097: Make the use of madvise() with MADV_DONTDU...

Posted by PSUdaemon <gi...@git.apache.org>.
Github user PSUdaemon commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1097#discussion_r83922132
  
    --- Diff: iocore/eventsystem/EventSystem.cc ---
    @@ -51,5 +52,15 @@ ink_event_system_init(ModuleVersion v)
       if (default_large_iobuffer_size > max_iobuffer_size) {
         default_large_iobuffer_size = max_iobuffer_size;
       }
    -  init_buffer_allocators();
    +
    +#ifdef MADV_DONTDUMP // This should only exist on Linux 3.4 and higher.
    +  RecBool dont_dump_enabled = true;
    +  RecGetRecordBool("proxy.config.allocator.dontdump_iobuffers", (RecBool *)&dont_dump_enabled, false);
    --- End diff --
    
    Is this case still needed?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1097: Make the use of madvise() with MADV_DONTDU...

Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1097#discussion_r83016016
  
    --- Diff: iocore/eventsystem/EventSystem.cc ---
    @@ -51,5 +51,15 @@ ink_event_system_init(ModuleVersion v)
       if (default_large_iobuffer_size > max_iobuffer_size) {
         default_large_iobuffer_size = max_iobuffer_size;
       }
    +
    +#ifdef MADV_DONTDUMP // This should only exist on Linux 3.4 and higher.
    +  bool dont_dump_enabled;
    --- End diff --
    
    Initialize to false.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1097: TS-4957: Make the use of madvise() with MA...

Posted by jrushford <gi...@git.apache.org>.
Github user jrushford closed the pull request at:

    https://github.com/apache/trafficserver/pull/1097


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1097: Make the use of madvise() with MADV_DONTDU...

Posted by PSUdaemon <gi...@git.apache.org>.
Github user PSUdaemon commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1097#discussion_r83040763
  
    --- Diff: iocore/eventsystem/I_IOBuffer.h ---
    @@ -58,6 +58,7 @@ class VIO;
     inkcoreapi extern int64_t max_iobuffer_size;
     extern int64_t default_small_iobuffer_size;
     extern int64_t default_large_iobuffer_size; // matched to size of OS buffers
    +extern int iobuffer_advice;
    --- End diff --
    
    I think what we ran into last time was a dependency ordering issue, which is why we scrapped the whole idea of a config variable in the first iteration.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1097: Make the use of madvise() with MADV_DONTDU...

Posted by jrushford <gi...@git.apache.org>.
Github user jrushford commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1097#discussion_r83292609
  
    --- Diff: iocore/eventsystem/EventSystem.cc ---
    @@ -51,5 +51,15 @@ ink_event_system_init(ModuleVersion v)
       if (default_large_iobuffer_size > max_iobuffer_size) {
         default_large_iobuffer_size = max_iobuffer_size;
       }
    +
    +#ifdef MADV_DONTDUMP // This should only exist on Linux 3.4 and higher.
    +  bool dont_dump_enabled;
    --- End diff --
    
    done


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver issue #1097: Make the use of madvise() with MADV_DONTDUMP conf...

Posted by jrushford <gi...@git.apache.org>.
Github user jrushford commented on the issue:

    https://github.com/apache/trafficserver/pull/1097
  
    [approve ci]


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1097: Make the use of madvise() with MADV_DONTDU...

Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1097#discussion_r83016299
  
    --- Diff: iocore/eventsystem/EventSystem.cc ---
    @@ -51,5 +51,15 @@ ink_event_system_init(ModuleVersion v)
       if (default_large_iobuffer_size > max_iobuffer_size) {
         default_large_iobuffer_size = max_iobuffer_size;
       }
    +
    +#ifdef MADV_DONTDUMP // This should only exist on Linux 3.4 and higher.
    +  bool dont_dump_enabled;
    --- End diff --
    
    Use RecBool for the type so it matches the cast below.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver issue #1097: Make the use of madvise() with MADV_DONTDUMP conf...

Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:

    https://github.com/apache/trafficserver/pull/1097
  
    Linux build *successful*! See https://ci.trafficserver.apache.org/job/Github-Linux/867/ for details.
     



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver issue #1097: Make the use of madvise() with MADV_DONTDUMP conf...

Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:

    https://github.com/apache/trafficserver/pull/1097
  
    FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/1046/ for details.
     



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver issue #1097: Make the use of madvise() with MADV_DONTDUMP conf...

Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:

    https://github.com/apache/trafficserver/pull/1097
  
    FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/1045/ for details.
     



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1097: Make the use of madvise() with MADV_DONTDU...

Posted by PSUdaemon <gi...@git.apache.org>.
Github user PSUdaemon commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1097#discussion_r82896873
  
    --- Diff: iocore/eventsystem/EventSystem.cc ---
    @@ -51,5 +51,15 @@ ink_event_system_init(ModuleVersion v)
       if (default_large_iobuffer_size > max_iobuffer_size) {
         default_large_iobuffer_size = max_iobuffer_size;
       }
    +
    +#ifdef MADV_DONTDUMP // This should only exist on Linux 3.4 and higher.
    +  bool dont_dump_enabled;
    +  RecGetRecordBool("proxy.config.allocator.dontdump_iobuffers", (RecBool *)&dont_dump_enabled, false);
    +
    +  if (dont_dump_enabled) {
    +    iobuffer_advice = MADV_DONTDUMP;
    --- End diff --
    
    Should we do `|=` incase we do more in the future?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver issue #1097: Make the use of madvise() with MADV_DONTDUMP conf...

Posted by jrushford <gi...@git.apache.org>.
Github user jrushford commented on the issue:

    https://github.com/apache/trafficserver/pull/1097
  
    @PSUdaemon - requested changes have been made and squashed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1097: Make the use of madvise() with MADV_DONTDU...

Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1097#discussion_r83016651
  
    --- Diff: iocore/eventsystem/I_IOBuffer.h ---
    @@ -58,6 +58,7 @@ class VIO;
     inkcoreapi extern int64_t max_iobuffer_size;
     extern int64_t default_small_iobuffer_size;
     extern int64_t default_large_iobuffer_size; // matched to size of OS buffers
    +extern int iobuffer_advice;
    --- End diff --
    
    We added another global variable? Can't we do this locally in ``init_buffer_allocators()``?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver issue #1097: Make the use of madvise() with MADV_DONTDUMP conf...

Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:

    https://github.com/apache/trafficserver/pull/1097
  
    Linux build *successful*! See https://ci.trafficserver.apache.org/job/Github-Linux/937/ for details.
     



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1097: Make the use of madvise() with MADV_DONTDU...

Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1097#discussion_r82917530
  
    --- Diff: mgmt/RecordsConfig.cc ---
    @@ -1449,6 +1449,8 @@ static const RecordElement RecordsConfig[] =
       ,
       {RECT_CONFIG, "proxy.config.allocator.hugepages", RECD_INT, "0", RECU_RESTART_TS, RR_NULL, RECC_NULL, "[0-1]", RECA_NULL}
       ,
    +  {RECT_CONFIG, "proxy.config.allocator.dontdump_iobuffers", RECD_INT, "0", RECU_RESTART_TS, RR_NULL, RECC_NULL, "[0-1]", RECA_NULL}
    --- End diff --
    
    I'm ok with that, but that means this also should be back ported to 7.0.0.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---