[GitHub] trafficserver pull request #1063: TS-4909: Throttling based on resident memo...

[bryancall]

Github user bryancall closed the pull request at:

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


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1063: TS-4909: Throttling based on resident memo...

[jpeach]

Github user jpeach commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/1063#discussion_r83504741
  
--- Diff: proxy/Main.cc ---
@@ -346,6 +346,54 @@ class DiagsLogContinuation : public Continuation
   }
 };
 
+class MemoryLimit : public Continuation
+{
+public:
+  MemoryLimit() : Continuation(new_ProxyMutex()), _memory_limit(0) { 
SET_HANDLER(::periodic); }
+  ~MemoryLimit() { mutex = NULL; }
+  int
+  periodic(int event, Event *e)
+  {
+if (event == EVENT_IMMEDIATE) {
+  // rescheduled from periodic to immediate event
+  // this is the indication to terminate
+  delete this;
+  return EVENT_DONE;
+}
+if (_memory_limit == 0) {
+  // first time it has been run
+  _memory_limit = 
REC_ConfigReadInteger("proxy.config.memory.max_usage");
--- End diff --

Ok that sounds reasonable.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1063: TS-4909: Throttling based on resident memo...

[jpeach]

Github user jpeach commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/1063#discussion_r83504537
  
--- Diff: iocore/net/UnixNetAccept.cc ---
@@ -280,6 +234,14 @@ NetAccept::do_blocking_accept(EThread *t)
   return -1;
 }
 
+// Throttle accepts
+if (!backdoor && (check_net_throttle(ACCEPT, now) || 
net_memory_throttle)) {
+  Debug("net_accept", "Too many connections or too much memory used, 
throttling");
+  check_throttle_warning();
+  con.close();
--- End diff --

Ok, sounds good.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1063: TS-4909: Throttling based on resident memo...

[bryancall]

Github user bryancall commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/1063#discussion_r83502802
  
--- Diff: iocore/net/UnixNetAccept.cc ---
@@ -280,6 +234,14 @@ NetAccept::do_blocking_accept(EThread *t)
   return -1;
 }
 
+// Throttle accepts
+if (!backdoor && (check_net_throttle(ACCEPT, now) || 
net_memory_throttle)) {
+  Debug("net_accept", "Too many connections or too much memory used, 
throttling");
+  check_throttle_warning();
+  con.close();
--- End diff --

check_throttle_warning logs an error message.  There currently isn't a stat 
for throttled incoming connections.  I can file a bug for it since I believe it 
will need to be added in a couple places.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1063: TS-4909: Throttling based on resident memo...

[bryancall]

Github user bryancall commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/1063#discussion_r83501005
  
--- Diff: proxy/Main.cc ---
@@ -346,6 +346,54 @@ class DiagsLogContinuation : public Continuation
   }
 };
 
+class MemoryLimit : public Continuation
+{
+public:
+  MemoryLimit() : Continuation(new_ProxyMutex()), _memory_limit(0) { 
SET_HANDLER(::periodic); }
+  ~MemoryLimit() { mutex = NULL; }
+  int
+  periodic(int event, Event *e)
+  {
+if (event == EVENT_IMMEDIATE) {
+  // rescheduled from periodic to immediate event
+  // this is the indication to terminate
+  delete this;
+  return EVENT_DONE;
+}
+if (_memory_limit == 0) {
+  // first time it has been run
+  _memory_limit = 
REC_ConfigReadInteger("proxy.config.memory.max_usage");
+}
+if (_memory_limit > 0) {
+  if (getrusage(RUSAGE_SELF, &_usage) == 0) {
+Debug("server", "memory usage - ru_maxrss: %ld memory limit: %" 
PRId64, _usage.ru_maxrss, _memory_limit);
+if (_usage.ru_maxrss > _memory_limit) {
--- End diff --

Fixed this. Did a divide by 1024 for the setting.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1063: TS-4909: Throttling based on resident memo...

[bryancall]

Github user bryancall commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/1063#discussion_r83498036
  
--- Diff: proxy/Main.cc ---
@@ -346,6 +346,54 @@ class DiagsLogContinuation : public Continuation
   }
 };
 
+class MemoryLimit : public Continuation
+{
+public:
+  MemoryLimit() : Continuation(new_ProxyMutex()), _memory_limit(0) { 
SET_HANDLER(::periodic); }
+  ~MemoryLimit() { mutex = NULL; }
+  int
+  periodic(int event, Event *e)
+  {
+if (event == EVENT_IMMEDIATE) {
+  // rescheduled from periodic to immediate event
+  // this is the indication to terminate
+  delete this;
+  return EVENT_DONE;
+}
+if (_memory_limit == 0) {
+  // first time it has been run
+  _memory_limit = 
REC_ConfigReadInteger("proxy.config.memory.max_usage");
--- End diff --

I don't have an event scheduled if it is not configured.  I thought it 
would be nice to not have an event scheduled every 1 second if someone doesn't 
have it configured.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1063: TS-4909: Throttling based on resident memo...

[jpeach]

Github user jpeach commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/1063#discussion_r82057095
  
--- Diff: proxy/Main.cc ---
@@ -346,6 +346,54 @@ class DiagsLogContinuation : public Continuation
   }
 };
 
+class MemoryLimit : public Continuation
+{
+public:
+  MemoryLimit() : Continuation(new_ProxyMutex()), _memory_limit(0) { 
SET_HANDLER(::periodic); }
+  ~MemoryLimit() { mutex = NULL; }
+  int
+  periodic(int event, Event *e)
+  {
+if (event == EVENT_IMMEDIATE) {
+  // rescheduled from periodic to immediate event
+  // this is the indication to terminate
+  delete this;
+  return EVENT_DONE;
+}
+if (_memory_limit == 0) {
+  // first time it has been run
+  _memory_limit = 
REC_ConfigReadInteger("proxy.config.memory.max_usage");
--- End diff --

Can you re-read this on each check so that it is a dynamic configuration?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1063: TS-4909: Throttling based on resident memo...

[jpeach]

Github user jpeach commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/1063#discussion_r82056583
  
--- Diff: iocore/net/UnixNetAccept.cc ---
@@ -280,6 +234,14 @@ NetAccept::do_blocking_accept(EThread *t)
   return -1;
 }
 
+// Throttle accepts
+if (!backdoor && (check_net_throttle(ACCEPT, now) || 
net_memory_throttle)) {
+  Debug("net_accept", "Too many connections or too much memory used, 
throttling");
+  check_throttle_warning();
+  con.close();
--- End diff --

Add a metric for this case?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1063: TS-4909: Throttling based on resident memo...

[jpeach]

Github user jpeach commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/1063#discussion_r82057266
  
--- Diff: proxy/Main.cc ---
@@ -346,6 +346,54 @@ class DiagsLogContinuation : public Continuation
   }
 };
 
+class MemoryLimit : public Continuation
+{
+public:
+  MemoryLimit() : Continuation(new_ProxyMutex()), _memory_limit(0) { 
SET_HANDLER(::periodic); }
+  ~MemoryLimit() { mutex = NULL; }
+  int
+  periodic(int event, Event *e)
+  {
+if (event == EVENT_IMMEDIATE) {
+  // rescheduled from periodic to immediate event
+  // this is the indication to terminate
+  delete this;
+  return EVENT_DONE;
+}
+if (_memory_limit == 0) {
+  // first time it has been run
+  _memory_limit = 
REC_ConfigReadInteger("proxy.config.memory.max_usage");
+}
+if (_memory_limit > 0) {
+  if (getrusage(RUSAGE_SELF, &_usage) == 0) {
+Debug("server", "memory usage - ru_maxrss: %ld memory limit: %" 
PRId64, _usage.ru_maxrss, _memory_limit);
+if (_usage.ru_maxrss > _memory_limit) {
--- End diff --

What units are we working with here? ``ru_maxrss`` is in KB, but one would 
think that ``proxy.config.memory.max_usage`` is in bytes? 
``proxy.config.memory.max_usage`` as bytes makes sense because then you can do 
``proxy.config.memory.max_usage=100GB``.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1063: TS-4909: Throttling based on resident memo...

[jpeach]

Github user jpeach commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/1063#discussion_r82056447
  
--- Diff: proxy/Main.cc ---
@@ -346,6 +346,54 @@ class DiagsLogContinuation : public Continuation
   }
 };
 
+class MemoryLimit : public Continuation
+{
+public:
+  MemoryLimit() : Continuation(new_ProxyMutex()), _memory_limit(0) { 
SET_HANDLER(::periodic); }
+  ~MemoryLimit() { mutex = NULL; }
+  int
+  periodic(int event, Event *e)
+  {
+if (event == EVENT_IMMEDIATE) {
+  // rescheduled from periodic to immediate event
+  // this is the indication to terminate
+  delete this;
+  return EVENT_DONE;
+}
+if (_memory_limit == 0) {
+  // first time it has been run
+  _memory_limit = 
REC_ConfigReadInteger("proxy.config.memory.max_usage");
+}
+if (_memory_limit > 0) {
+  if (getrusage(RUSAGE_SELF, &_usage) == 0) {
+Debug("server", "memory usage - ru_maxrss: %ld memory limit: %" 
PRId64, _usage.ru_maxrss, _memory_limit);
+if (_usage.ru_maxrss > _memory_limit) {
+  if (net_memory_throttle == false) {
+net_memory_throttle = true;
+Debug("server", "memory usage exceeded limit - ru_maxrss: %ld 
memory limit: %" PRId64, _usage.ru_maxrss, _memory_limit);
+  }
+} else {
+  if (net_memory_throttle == true) {
+net_memory_throttle = false;
+Debug("server", "memory usage under limit - ru_maxrss: %ld 
memory limit: %" PRId64, _usage.ru_maxrss, _memory_limit);
+  }
+}
+  }
+} else {
+  // this feature has not be enabled
+  Debug("server", "limiting connections baxed on memory usage has been 
disabled");
--- End diff --

s/baxed/based/


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1063: TS-4909: Throttling based on resident memo...

[bryancall]

GitHub user bryancall opened a pull request:

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

TS-4909: Throttling based on resident memory usage



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

$ git pull https://github.com/bryancall/trafficserver TS-4909

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

https://github.com/apache/trafficserver/pull/1063.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 #1063


commit 66c79b32efcd2eac8da8c631f89d20681617e751
Author: Bryan Call 
Date:   2016-09-29T04:17:43Z

TS-4909: Throttling based on resident memory




---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---