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.

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

add idle_timeout to fn tool #663

Merged
merged 7 commits into from
Jan 26, 2018
Merged

add idle_timeout to fn tool #663

merged 7 commits into from
Jan 26, 2018

Conversation

c0ze
Copy link
Contributor

@c0ze c0ze commented Jan 5, 2018

It seems fn tool doesn't support setting idle_timeout param for hot function routes.

https://github.com/iron-io/functions/blob/master/docs/function-timeouts.md
https://github.com/iron-io/functions/blob/master/docs/hot-functions.md

To test, create an app, and a route with :

go run fn/main.go routes c --format=http --memory=2048 --timeout=60s --idle_timeout=60s hot-func /hot

modify hot-functions func.go to look something like :

func main() {
	var pre []byte

	for {
		res := http.Response{
			Proto:      "HTTP/1.1",
			ProtoMajor: 1,
			ProtoMinor: 1,
			StatusCode: 200,
			Status:     "OK",
		}

		r := bufio.NewReader(os.Stdin)
		req, err := http.ReadRequest(r)

		var buf bytes.Buffer
		if err != nil {
			res.StatusCode = 500
			res.Status = http.StatusText(res.StatusCode)
			fmt.Fprintln(&buf, err)
		} else {
			l, _ := strconv.Atoi(req.Header.Get("Content-Length"))
			p := make([]byte, l)
			r.Read(p)
			fmt.Fprintf(&buf, "Previous call was %s\n", pre)
			fmt.Fprintf(&buf, "Hello %s\n", p)
			for k, vs := range req.Header {
				fmt.Fprintf(&buf, "ENV: %s %#v\n", k, vs)
			}
			pre = p
		}

		res.Body = ioutil.NopCloser(&buf)
		res.ContentLength = int64(buf.Len())
		res.Write(os.Stdout)
	}
}

to keep a passed value, and print both when there is a request. Then you can call the function with :

curl -X POST localhost:8080/r/hot-func/hot -d 'Test'
curl -X POST localhost:8080/r/hot-func/hot -d 'Test 2'

You should see previous call in the second call output.

after a while, you should see the hot func spin down in the service logs

INFO[0142] Canceling inactive hot function               app=hot-func format=http idle_timeout=1m0s image="coze/hot:0.0.1" max_concurrency=1 memory=2048 route="/hot"
INFO[0142] container status                              container_error= container_finished=0001-01-01 00:00:00 +0000 UTC container_running=true container_status=running exit_code=0
INFO[0142]                                               name=run.hot-func.error type=count value=1
INFO[0142]                                               name=run.hot-func.time type=time value=1m3.2915359s
INFO[0142]                                               name="run.exec_time" type=time value=1m3.2915359s
INFO[0143] hot function terminated                       result=

without this patch (and setting idle_timeout) the function spins down immediately after serving the response.

Copy link
Contributor

@kunihiko-t kunihiko-t left a comment

Choose a reason for hiding this comment

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

LGTM 👍
I tested it with your command and func.yaml way and both of them succeeded.

Copy link

@vasilev vasilev left a comment

Choose a reason for hiding this comment

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

Looks good to me too.
Any chance to cover this with an autotest ?

@c0ze
Copy link
Contributor Author

c0ze commented Jan 5, 2018

@kunihiko-t @vasilev

Thanks ! Yes I am looking at providing some test coverage. Might take a while !

@c0ze c0ze changed the title add idle_timeout to fn tool [WIP] add idle_timeout to fn tool Jan 5, 2018
@c0ze c0ze mentioned this pull request Jan 14, 2018
5 tasks
@iron-io iron-io deleted a comment from kunihiko-t Jan 16, 2018
@c0ze
Copy link
Contributor Author

c0ze commented Jan 25, 2018

@kunihiko-t @vasilev

I added tests for setting idle_timeout (and every other param) from fn tool ! I would appreciate if you could take a look !

Regarding the test I posted go code above, it actually tests hot functions spin down behaviour, which is beyond the scope of this PR. This PR only deals with setting idle_timeout from fn tool.

Copy link

@vasilev vasilev left a comment

Choose a reason for hiding this comment

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

Looks good to me.
Thanks for tests!

Copy link
Contributor

@kunihiko-t kunihiko-t left a comment

Choose a reason for hiding this comment

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

Thank you! LGTM 👍

@@ -43,6 +43,7 @@ type Funcfile struct {
Memory *int64 `yaml:"memory,omitempty" json:"memory,omitempty"`
Format *string `yaml:"format,omitempty" json:"format,omitempty"`
Timeout *time.Duration `yaml:"timeout,omitempty" json:"timeout,omitempty"`
IDLETimeout *time.Duration `yaml:"idle_timeout,omitempty" json:"idle_timeout,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

nits: I think IdleTimeout is better than IDLETimeout.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes but unfortunately it is defined as IDLETimeout in functions_go (which is auto generated). I am not sure why it capitalizes.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, so it's no problem.
Thanks 👍

@sss0350
Copy link

sss0350 commented Jan 26, 2018

I'm checking your hotfunction example and trying to make it running on my environment.
(https://github.com/iron-io/functions/tree/master/examples/hotfunctions/http)
But it seems the idle_timeout is not possible setting from fn cmd.
I set it in func.yaml ( ex: idle_timeout:30 ) , and inpect it using fn routes inspect. But it always get 0.

And find this post , it seems you guys are working on it . Let me know when there's a patch of fn cmd.
And will be really nice if user can set on function-ui.

Just let you guys know my result. Thanks again.

@c0ze
Copy link
Contributor Author

c0ze commented Jan 26, 2018

@sss0350 Thanks for the response !

Yes, this is actually ready to go, so setting idle_timeout from fn will be available in a few moments. (it will be version 0.2.72, so dont forget to update !)

Regarding UI, we already added it but it is not released yet. We will combine a couple of feature requests for UI and release a new version soon (hopefully in a couple of weeks) !

@c0ze c0ze merged commit 59d9689 into master Jan 26, 2018
@c0ze c0ze changed the title [WIP] add idle_timeout to fn tool add idle_timeout to fn tool Jan 26, 2018
@sss0350
Copy link

sss0350 commented Jan 28, 2018

I just get it a quick try this afternoon at my office, I can set idle_timeout from cmd.

  1. fn routes update xxx xxx --idle_timeout=30s
  2. fn routes inspect xxx xxx (can see idle_timeout set to 30s)

And trigger your hot function examples ( https://github.com/iron-io/functions/tree/master/examples/hotfunctions/http ), but find the containers still not keep alive within 30s.
After trigger function , and I check this hot function container thru docker ps | grep myfnname ,
it seems this container still be killed immediately after trigger. It should be alive within 30s , right?
Did I misunderstand anything here? Or any other steps should be done to make a hotfunction?

@c0ze
Copy link
Contributor Author

c0ze commented Jan 28, 2018

Yes, it should be alive for the duration of idle timeout. You can see if it's being killed in the server logs. Can you see anything there?

@c0ze c0ze deleted the add-idle-timeout-to-fn branch January 28, 2018 10:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants