Repository: istio/fortio
Pull request #190 (2018-03-07 19:42:46)

add MaxPayloadSizeKb parameter to set max payload size for echo call. eg. echo/?size=
wrt: https://github.com/istio/fortio/issues/183


ldemailly: glad the tests work as I expect:
```
--- FAIL: TestGenerateSize (0.00s)
	http_test.go:488: Got 262144, expected 256 for generateSize("262145")
```

protip : as per readme, run "make test webtest lint" before submitting a PR
kidiyoor: ```var payload = make([]byte, MaxPayloadSizeKb*1024)``` issue is still not addressed. 
kidiyoor: latest commit should fix that issue. However test not added yet.
kidiyoor: I'm sure something is still missing :?)
ldemailly: I split code into more manageable chunks/files - sorry you need to rebase/have a conflict
ldemailly: ping :)
kidiyoor: Pong. I had given a little pause on this.  
Expect few more commits soon enough.

Patch:
diff --git a/README.md b/README.md
index 880ad4a..0a1251d 100644
--- a/README.md
+++ b/README.md
@@ -112,6 +112,8 @@ and flags are:
       (default Info)
   -logprefix string
     	Prefix to log lines before logged messages (default "> ")
+  -maxpayloadsizekb int
+    	MaxPayloadSizeKb is the maximum size of payload to be generated by the EchoHandler size= argument (In kilobytes defaults to 256)
   -n int
     	Run for exactly this number of calls instead of duration. Default (0) is
     	to use duration (-t). Default is 1 when used as grpc ping count.
diff --git a/fhttp/http_client.go b/fhttp/http_client.go
index 1f47e8a..02d6d61 100644
--- a/fhttp/http_client.go
+++ b/fhttp/http_client.go
@@ -44,7 +44,7 @@ type Fetcher interface {
 }
 
 var (
-	// BufferSizeKb size of the buffer (max data) for optimized client in kilobytes defaults to 32k.
+	// BufferSizeKb size of the buffer (max data) for optimized client in kilobytes defaults to 128k.
 	BufferSizeKb = 128
 	// CheckConnectionClosedHeader indicates whether to check for server side connection closed headers.
 	CheckConnectionClosedHeader = false
diff --git a/fhttp/http_server.go b/fhttp/http_server.go
index 25b6cf4..e005792 100644
--- a/fhttp/http_server.go
+++ b/fhttp/http_server.go
@@ -41,6 +41,11 @@ var (
 	startTime time.Time
 	// EchoRequests is the number of request received. Only updated in Debug mode.
 	EchoRequests int64
+	// MaxPayloadSize is the maximum size of payload to be generated by the
+	// EchoHandler size= argument. In bytes.
+	MaxPayloadSize = 256 * 1024
+	// Payload that is returned during echo call
+	payload []byte
 )
 
 // EchoHandler is an http server handler echoing back the input.
@@ -97,17 +102,25 @@ func EchoHandler(w http.ResponseWriter, r *http.Request) {
 	}
 }
 
-// MaxPayloadSize is the maximum size of payload to be generated by the
-// EchoHandler size= argument. In bytes.
-const MaxPayloadSize = 256 * 1024 // 256k
-
-var payload = make([]byte, MaxPayloadSize)
-
 func init() {
+	ChangeMaxPayloadSize(MaxPayloadSize)
+}
+
+// ChangeMaxPayloadSize is used to change max payload size and fill it with pseudorandom content
+func ChangeMaxPayloadSize(newMaxPayloadSize int) {
+	if newMaxPayloadSize >= 0 {
+		MaxPayloadSize = newMaxPayloadSize
+	} else {
+		MaxPayloadSize = 0
+	}
+	payload = make([]byte, MaxPayloadSize)
 	// One shared and 'constant' (over time) but pseudo random content for payload
 	// (to defeat compression). We don't need crypto strength here, just low cpu
 	// and speed:
-	rand.Read(payload) // nolint:gas
+	_, err := rand.Read(payload)
+	if err != nil {
+		log.Errf("Error changing paylaod size, while generating random paylaod")
+	}
 }
 
 func writePayload(w http.ResponseWriter, status int, size int) {
diff --git a/fortio_main.go b/fortio_main.go
index 0597e4e..24a6d43 100644
--- a/fortio_main.go
+++ b/fortio_main.go
@@ -114,6 +114,8 @@ var (
 	syncFlag    = flag.String("sync", "", "index.tsv or s3/gcs bucket xml URL to fetch at startup for server modes.")
 	baseURLFlag = flag.String("base-url", "",
 		"base URL used as prefix for data/index.tsv generation. (when empty, the url from the first request is used)")
+	newMaxPayloadSizeKb = flag.Int("maxpayloadsizekb", fhttp.MaxPayloadSize/1024,
+		"MaxPayloadSize is the maximum size of payload to be generated by the EchoHandler size= argument. In Kbytes.")
 
 	// GRPC related flags
 	// To get most debugging/tracing:
@@ -145,6 +147,7 @@ func main() {
 	command := os.Args[1]
 	os.Args = append([]string{os.Args[0]}, os.Args[2:]...)
 	flag.Parse()
+	fhttp.ChangeMaxPayloadSize(*newMaxPayloadSizeKb * 1024)
 	if *quietFlag {
 		log.SetLogLevelQuiet(log.Error)
 	}


Test patch:
diff --git a/Webtest.sh b/Webtest.sh
index 70a0b81..c3008d1 100755
--- a/Webtest.sh
+++ b/Webtest.sh
@@ -5,8 +5,9 @@ make docker-internal TAG=webtest || exit 1
 FORTIO_UI_PREFIX=/newprefix/ # test the non default prefix (not /fortio/)
 FILE_LIMIT=20 # must be low to detect leaks
 LOGLEVEL=info # change to debug to debug
+MAXPAYLOAD=8 # Max Payload size for echo?size= in kb
 DOCKERNAME=fortio_server
-DOCKERID=$(docker run -d --ulimit nofile=$FILE_LIMIT --name $DOCKERNAME istio/fortio:webtest server -ui-path $FORTIO_UI_PREFIX -loglevel $LOGLEVEL)
+DOCKERID=$(docker run -d --ulimit nofile=$FILE_LIMIT --name $DOCKERNAME istio/fortio:webtest server -ui-path $FORTIO_UI_PREFIX -loglevel $LOGLEVEL -maxpayloadsizekb $MAXPAYLOAD)
 function cleanup {
   docker stop $DOCKERID
   docker rm $DOCKERNAME
@@ -41,6 +42,14 @@ if [ "$SIZE" -lt 50000 ]; then
   echo "Too small fetch for js: $SIZE"
   exit 1
 fi
+# Check if max payload set to value passed in cmd line parameter -maxpayloadsizekb
+SIZE=$($CURL "${BASE_URL}/echo?size=1048576" |wc -c)
+# Payload is 8192 but between content chunking and headers fast client can return up to 8300 or so
+if [ "$SIZE" -lt 8191 ] || [ "$SIZE" -gt 8400 ]; then
+  echo "-maxpayloadsizekb not working as expected"
+  exit 1
+fi
+
 # Check the main page
 $CURL $BASE_FORTIO
 # Do a small http load using std client
diff --git a/fhttp/http_test.go b/fhttp/http_test.go
index 0408ad5..a8dda29 100644
--- a/fhttp/http_test.go
+++ b/fhttp/http_test.go
@@ -800,3 +800,25 @@ func BenchmarkFoldFind(b *testing.B) {
 		FoldFind(testHaystack, needle)
 	}
 }
+
+func TestChangeMaxPayloadSize(t *testing.T) {
+	var tests = []struct {
+		input    int
+		expected int
+	}{
+		// negative test cases
+		{-1, 0},
+		// lesser than current default
+		{0, 0},
+		{64, 64},
+		// Greater than current default
+		{987 * 1024, 987 * 1024},
+	}
+	for _, tst := range tests {
+		ChangeMaxPayloadSize(tst.input)
+		actual := len(payload)
+		if len(payload) != tst.expected {
+			t.Errorf("Got %d, expected %d for ChangeMaxPayloadSize(%d)", actual, tst.expected, tst.input)
+		}
+	}
+}