nsqd: Make REQ timeout limit configurable Fixes #348 #352
Conversation
|
Ready for review |
| @@ -41,6 +41,7 @@ var ( | |||
| msgTimeout = flagSet.String("msg-timeout", "60s", "duration to wait before auto-requeing a message") | |||
| maxMsgTimeout = flagSet.Duration("max-msg-timeout", 15*time.Minute, "maximum duration before a message will timeout") | |||
| maxMsgSize = flagSet.Int64("max-msg-size", 1024768, "maximum size of a single message in bytes") | |||
| maxReqTimeout = flagSet.Duration("max-req-timeout", 1*time.Hour, "maximum requeueing timeout for a message") | |||
There was a problem hiding this comment.
@AlphaB want to fix up that typo?
|
@AlphaB cool, thanks for putting this together would you mind squashing down to one commit? |
|
Done =) |
|
thanks! |
nsqd: Make REQ timeout limit configurable Fixes #348
|
cool but one hour seems pretty arbitrary, I think that's why I'm not really a fan of some of these upper limits, if you have a bunch of nsqd's deployed then it just becomes a bit of a burden when you realize one of the arbitrary max's wasn't ideal |
|
For the record it was already hard-coded at one hour, now at least you can control it. The philosophy is leave nothing unbounded (where possible), choose sane defaults, and provide the ability to configure it how you want. I think it's important that you make explicit choices to change certain settings. I'm all ears if you think we've done a poor job with arbitrary defaults that aren't sane. For some context on this specific setting, requeued messages that are deferred are kept in-memory (see issue #34, the oldest open issue 😁). So if we did not apply this limit it essentially opens up the door to unexpected unbounded memory growth. I would argue that it should be lower than 1hr! |
|
Ahhh ok interesting yeah that could be bad haha, I just found myself setting pretty high limits for everything out the gate so I have that choice in the client, but it's not a huge deal either way, not hard to do that |
Fixes #348