Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add message streaming mode #1012

Closed
wants to merge 26 commits into from
Closed

Add message streaming mode #1012

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
9c5b992
cfg: add parse long and unify error messages on parse integer errors
vankoven Aug 27, 2018
ff5d793
fix incorrect spelling
vankoven Aug 27, 2018
2bfaeaf
restrict client rmem size
vankoven Aug 27, 2018
a1639f0
export sysctl_tcp_moderate_rcvbuf in kernel
vankoven Aug 27, 2018
989dfb4
add missing state in TCP debug
vankoven Aug 27, 2018
fd67725
Add client|server_msg_buffering configuration directive
vankoven Aug 27, 2018
22481bb
move http parser functions to separate header
vankoven Aug 27, 2018
53161dd
move parser to connetion, don't allocate it for each incoming message
vankoven Aug 29, 2018
b2c643d
use standart bitops functions for message flags.
vankoven Aug 29, 2018
7fa3d1a
Correct more spelling mistakes
vankoven Aug 29, 2018
b5bb500
Remove duplicated namings for connection flags
vankoven Aug 29, 2018
1f3542b
move comment to single line
vankoven Aug 29, 2018
8846486
remove extra type conversions
vankoven Aug 29, 2018
9d5a3f0
TODO client_responses_rmem
vankoven Aug 29, 2018
3f9693f
Remove TFW_HTTP_SUSPECTED flag
vankoven Aug 29, 2018
cd02fcf
unify tfw_http_cli_error_resp_and_log code
vankoven Aug 29, 2018
8ad0e02
reformat tfw_http_req_process to simplify connection management
vankoven Aug 29, 2018
8a8a20b
Add todo for tfw_http_cli_error_resp_and_log
vankoven Sep 4, 2018
9921c7b
reformat tfw_http_resp_process to simplify connection management
vankoven Aug 29, 2018
6a613ce
correctly delist request on response parsing errors
vankoven Aug 29, 2018
63054ab
TFW_HTTP_FSM_LOCAL_RESP_FILTER is egress state, don't mix up with ing…
vankoven Aug 29, 2018
54c8ceb
correct spelling mistakes
vankoven Aug 29, 2018
e8e111b
don't break logged line
vankoven Aug 29, 2018
f696de7
parser: add helper function to allow processing of streamed parts
vankoven Aug 29, 2018
1df138b
unify tfw_http_cli_error_resp_and_log code
vankoven Aug 29, 2018
b51dcca
Move msg_sent to TFW_CONN_COMMON
vankoven Aug 29, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 81 additions & 10 deletions etc/tempesta_fw.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@
# Default:
# None.

#
# TAG: nonidempotent
#
# Defines a request that is considered non-idempotent.
Expand All @@ -116,7 +115,6 @@
# A non-idempotent request in defined as a request that has an unsafe method.
#

#
# TAG: server_connect_retries
#
# Defines the maximum number of attempts to reconnect with a server.
Expand All @@ -133,7 +131,6 @@
# server_connect_retries 10;
#

#
# TAG: server_forward_retries
#
# Defines the maximum number of attempts to re-forward a request to a server.
Expand All @@ -145,8 +142,6 @@
# server_forward_retries 5;
#


#
# TAG: server_forward_timeout
#
# Defines the maximum time frame within which a request may still be forwarded.
Expand All @@ -158,7 +153,6 @@
# server_forward_timeout 60;
#

#
# TAG: server_retry_nonidempotent
#
# Defines that non-idempotent requests should be re-forwarded.
Expand All @@ -170,7 +164,6 @@
# Do not re-forward non-idempotent requests.
#

#
# TAG: server_queue_size
#
# Defines the size of the forwarding queue in a server connection.
Expand All @@ -185,7 +178,6 @@
# server_queue_size 1000;
#

#
# TAG: grace_shutdown_time
#
# Defines the timeout in seconds for graceful server removing during
Expand Down Expand Up @@ -408,7 +400,7 @@
# keepalive_timeout 75;

# TAG: listen
#
#
# Tempesta FW listening address.
#
# Syntax:
Expand All @@ -435,6 +427,85 @@
# Default:
# listen 80;

# TAG: server_msg_buffering
#
# Mode of operation: buffering or streaming.
#
# Syntax:
# server_msg_buffering SIZE
#
# In buffering mode the whole message is assembled on Tempesta's side before
# forwarding to client. In streaming mode a new message part is streamed
# to client as soon it's received, full message is not stored in Tempesta's
# memory.
#
# SIZE - 64-bit integer value. May have following values:
# 0 - pure steaming mode. Only message headers are buffered;
# 1-.. - mixed mode: only first SIZE bytes of message body is buffered;
# -1 - pure buffering mode. The whole message is buffered before forwarding.
#
# Default:
# server_msg_buffering first 10MB of message, stream the rest:
# server_msg_buffering 10485760;

# TAG: client_msg_buffering
#
# Mode of operation: buffering or streaming.
#
# Syntax:
# client_msg_buffering SIZE
#
# In buffering mode the whole message is assembled on Tempesta's side before
# forwarding to backend server. In streaming mode a new message part is
# streamed to backend server as soon it's received, full message is not stored
# in Tempesta's memory
#
# SIZE - 64-bit integer value. May have following values:
# 0 - pure steaming mode. Only message headers are buffered;
# 1-.. - mixed mode: only first SIZE bytes of message body is buffered;
# -1 - pure buffering mode. The whole message is buffered before forwarding.
#
# Default:
# client_msg_buffering first 1MB of message, stream the rest:
# client_msg_buffering 1048576;

# TAG: client_rmem
#
# Limit memory used to store all requests by single client connection.
#
# Syntax:
# client_rmem SIZE
#
# Request has to be stored in Tempesta's buffers until the response is
# forwarded to the client. The option controls how many memory is used to
# store all requests for a single client connection. Tempesta uses
# TCP receive window steering and doesn't allow clients push more data
# than allowed. The value dosn't overload 'net.ipv4.tcp_rmem' limits.
#
# SIZE - buffer size in bytes, 32-bit integer value. '0' value means the option
# is disabled.
#
# Default:
# client_rmem 0;

# TAG: client_responses_rmem
#
# Limit memory used to store all responses for single client connection.
#
# Syntax:
# client_responses_rmem SIZE
#
# Responses has to be stored in Tempesta's buffers until they are forwarded
# to the client. The option controls how many memory is used to
# store all responses for a single client connection. Shutdown the client
# connection if memory consumption oferflows the limit.
#
# SIZE - buffer size in bytes, 32-bit integer value. '0' value means the option
# is disabled.
#
# Default:
# client_responses_rmem 0;

# TAG: block_action
#
# Syntax:
Expand Down Expand Up @@ -500,7 +571,7 @@
# cache 2;

# TAG: cache_db
#
#
# Path to a cache database used as a storage for Tempesta FW Web cache.
#
# Syntax:
Expand Down
29 changes: 9 additions & 20 deletions tempesta_fw/apm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
* You should have received a copy of the GNU General Public License along
* with this program; if not, write to the Free Software Foundation, Inc.,
* 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
* Prototype for fast precentiles calculation.
* Prototype for fast percentiles calculation.
*/
#include <linux/atomic.h>
#include <linux/kernel.h>
Expand Down Expand Up @@ -44,7 +44,7 @@
* 3. Very small overall memory footprint for inexpensive handling of
* performance trends of many servers;
*
* 4. Buckets must be dynamicaly rearranged since server response times
* 4. Buckets must be dynamically rearranged since server response times
* are unknown apriori;
*
* 5. The adjustments of buckets must be performed in a lock-less fashion
Expand Down Expand Up @@ -182,7 +182,7 @@ tfw_stats_extend(TfwPcntRanges *rng, unsigned int r_time)
* the upper end of the range that the data type could hold.
* As the value was extended to the next order it's conceivable
* that the new value exceeded the maximum for the data type.
* Consirering that TfwPcntCtl{}->end is of type unsigned int,
* Considering that TfwPcntCtl{}->end is of type unsigned int,
* it's totally unimaginable that this situation may ever happen.
*/
BUG_ON(end >= (1UL << (FIELD_SIZEOF(TfwPcntCtl, end) * 8)));
Expand Down Expand Up @@ -456,7 +456,7 @@ typedef struct {
* @rcount - current count of health monitoring requests (in @hm->tmt);
* @jtmstamp - time in jiffies of last @timer call (for procfs);
* @timer - timer for sending health monitoring request;
* @rearm - flag for gracefull stopping of @timer;
* @rearm - flag for graceful stopping of @timer;
*/
typedef struct {
TfwApmHM *hm;
Expand Down Expand Up @@ -514,7 +514,7 @@ typedef struct {
} TfwApmRBuf;

/*
* The ring buffer contol structure.
* The ring buffer control structure.
*
* This is a supporting structure. It keeps related data that is useful
* in making decisions on the need of recalculation of percentiles.
Expand Down Expand Up @@ -1693,19 +1693,13 @@ tfw_cfgop_apm_server_failover(TfwCfgSpec *cs, TfwCfgEntry *ce)
ce->vals[0]);
return -EINVAL;
}
if (tfw_cfg_parse_int(ce->vals[1], &limit)) {
TFW_ERR_NL("Unable to parse http limit value: '%s'\n",
ce->vals[1]);
if (tfw_cfg_parse_int(ce->vals[1], &limit))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, the PR is still unfinished and I can not run Tempesta with error configuration to see what it prints, but it seems that now it just prints that it can not parse some invalid integer, w/o localization which exactly configuration option causes the problem. If a user specifies a string for several configuration options and only one of them is actually an integer, then the user must check documentation for all the options and that's not user friendly. I think the unification of error messages in 71043eb is wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really like how Tempesta complains about parsing errors, e.g. for line server 127.0.0.1:8080 conns_n=1a;:

On master:

[tempesta] ERROR: Invalid value: '1a'
[tempesta] ERROR: configuration parsing error:
   5: server 127.0.0.1:8080 conns_n=1a;
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

On my branch:

[tempesta] ERROR: Can't parse '1a' as 'int'
[tempesta] ERROR: configuration parsing error:
   5: server 127.0.0.1:8080 conns_n=1a;
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

As for me, last option is more verbose. Yes, that commit made the output less verbose for some options. But before changes it looked ugly: every directive has it's own unique error message, error processing is also unique for every directive. Some errors described very good, while descriptions for others are completely missed. My variant is much easier to support.

The thing I completely dislike in parsing errors is reversed error message. Ideal message for me is below:

[tempesta] ERROR: configuration parsing error:
   5: server 127.0.0.1:8080 conns_n=1a;
                                    ^^
[tempesta] ERROR: Can't parse '1a' as 'int', allowed values are [1, 65535]

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, the change is actually good since we still have

   5: server 127.0.0.1:8080 conns_n=1a;
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

And yes,

   5: server 127.0.0.1:8080 conns_n=1a;
                                    ^^

would look much better.

return -EINVAL;
}
if (tfw_cfg_check_range(limit, 1, USHRT_MAX))
return -EINVAL;

if (tfw_cfg_parse_int(ce->vals[2], &tframe)) {
TFW_ERR_NL("Unable to parse http tframe value: '%s'\n",
ce->vals[2]);
if (tfw_cfg_parse_int(ce->vals[2], &tframe))
return -EINVAL;
}
if (tfw_cfg_check_range(tframe, 1, USHRT_MAX))
return -EINVAL;

Expand Down Expand Up @@ -1851,10 +1845,8 @@ tfw_cfgop_apm_hm_resp_crc32(TfwCfgSpec *cs, TfwCfgEntry *ce)
return 0;
}

if (tfw_cfg_parse_uint(ce->vals[0], &crc32)) {
TFW_ERR_NL("Unable to parse crc32 value: '%s'\n", ce->vals[0]);
if (tfw_cfg_parse_uint(ce->vals[0], &crc32))
return -EINVAL;
}

tfw_hm_entry->crc32 = crc32;

Expand All @@ -1868,11 +1860,8 @@ tfw_cfgop_apm_hm_timeout(TfwCfgSpec *cs, TfwCfgEntry *ce)

if (tfw_cfg_check_single_val(ce))
return -EINVAL;
if (tfw_cfg_parse_int(ce->vals[0], &timeout)) {
TFW_ERR_NL("Unable to parse http timeout value: '%s'\n",
ce->vals[0]);
if (tfw_cfg_parse_int(ce->vals[0], &timeout))
return -EINVAL;
}
if (tfw_cfg_check_range(timeout, 1, USHRT_MAX))
return -EINVAL;

Expand Down
Loading