You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficserver.apache.org by bryancall <gi...@git.apache.org> on 2016/09/29 04:23:41 UTC
[GitHub] trafficserver pull request #1063: TS-4909: Throttling based on resident memo...
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 <bc...@apache.org>
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1063: TS-4909: Throttling based on resident memory usag...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1063
Linux build *successful*! See https://ci.trafficserver.apache.org/job/Github-Linux/911/ 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 #1063: TS-4909: Throttling based on resident memo...
Posted by bryancall <gi...@git.apache.org>.
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(&MemoryLimit::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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1063: TS-4909: Throttling based on resident memory usag...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1063
FreeBSD build *failed*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/1020/ 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 #1063: TS-4909: Throttling based on resident memo...
Posted by jpeach <gi...@git.apache.org>.
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(&MemoryLimit::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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1063: TS-4909: Throttling based on resident memory usag...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1063
Linux build *failed*! See https://ci.trafficserver.apache.org/job/Github-Linux/912/ 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 #1063: TS-4909: Throttling based on resident memory usag...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1063
FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/900/ 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 #1063: TS-4909: Throttling based on resident memo...
Posted by jpeach <gi...@git.apache.org>.
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(&MemoryLimit::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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1063: TS-4909: Throttling based on resident memory usag...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1063
FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/1019/ 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 #1063: TS-4909: Throttling based on resident memo...
Posted by jpeach <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1063: TS-4909: Throttling based on resident memo...
Posted by jpeach <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1063: TS-4909: Throttling based on resident memo...
Posted by jpeach <gi...@git.apache.org>.
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(&MemoryLimit::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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1063: TS-4909: Throttling based on resident memo...
Posted by jpeach <gi...@git.apache.org>.
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(&MemoryLimit::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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1063: TS-4909: Throttling based on resident memory usag...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1063
Linux build *successful*! See https://ci.trafficserver.apache.org/job/Github-Linux/795/ 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 #1063: TS-4909: Throttling based on resident memo...
Posted by bryancall <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1063: TS-4909: Throttling based on resident memo...
Posted by bryancall <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1063: TS-4909: Throttling based on resident memo...
Posted by bryancall <gi...@git.apache.org>.
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(&MemoryLimit::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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---