Fix restore functionality hangs forever

[oiooj]

if return errors.New("listener closing") , the serveExecListenerfunction will not return in a loop，and the program will hangs at https://github.com/influxdb/influxdb/blob/master/meta/store.go#L567

Fix #4806

Ref: https://github.com/influxdb/influxdb/blob/master/meta/store.go#L739

• CHANGELOG.md updated
• Rebased/mergable
• Tests pass
• Sign CLA

[oiooj]

@pauldix @otoolep

[benbjohnson]

lgtm, although we should probably change the Accept() error check to look for IsTemporary() instead of doing string comparisons.

[otoolep]

I agree with @benbjohnson

@oiooj -- thanks, you have found a real bug in our code. However, the better fix is as shown here:

https://github.com/influxdb/influxdb/blob/master/services/opentsdb/service.go#L211

Would you mind following this pattern and fixing the bug that way? We shouldn't have coded string-compares.

[oiooj]

@otoolep fixed

[otoolep]

This doesn't look right to me.

If the error is not temporary, then simply return. Otherwise continue in the loop. Won't that work?

Also, why are you sleeping for a second? Is that debug code?

[oiooj]

@otoolep If the error is temporary, then continue. Other error return. if error == nil , handle this Conn.
And there is no need to sleep for a second, deleted.

[otoolep]

OK, I see where you are going.

This still seems a little bit awkward to me. Let me explain.

If the error returned from Accept is temporary, then keep looping. However, if the error is not temporary, then return. The way other code (code that is shutting down the system) signals that the function should return is simply by closing the listener. That will result in a non-temporary error, so polling closing isn't really necessary.

Perhaps this wouldn't work with the ExecListener for some reason? (I have not looked at that code yet).

[otoolep]

I would expect that if a signal is sent on closing then the ExecListener would be closed from somewhere else and a non-temporary error would result. I might be wrong though.

[oiooj]

@otoolep yes, you're right.

Now if I send a signal to shutdown the system all is normal.

[run] 2015/11/29 11:17:23 Signal received, initializing clean shutdown...
[run] 2015/11/29 11:17:23 Waiting for clean shutdown...
[metastore] 2015/11/29 11:17:23 RPC listener accept error and closed: network connection closed
[metastore] 2015/11/29 11:17:23 exec listener accept error and closed: network connection closed
[copier] 2015/11/29 11:17:23 copier listener closed
[cluster] 2015/11/29 11:17:23 cluster service accept error: network connection closed
[snapshot] 2015/11/29 11:17:23 snapshot listener closed
[shard-precreation] 2015/11/29 11:17:23 Precreation service terminating
[registration] 2015/11/29 11:17:23 registration service terminating
[continuous_querier] 2015/11/29 11:17:23 continuous query service terminating
[retention] 2015/11/29 11:17:23 retention policy enforcement terminating
[monitor] 2015/11/29 11:17:23 shutting down monitor system
[monitor] 2015/11/29 11:17:23 terminating storage of statistics
[handoff] 2015/11/29 11:17:23 shutting down hh service
[subscriber] 2015/11/29 11:17:23 closed service
[run] 2015/11/29 11:17:23 server shutdown completed

just like the cluster service.

[otoolep]

Thanks @oiooj -- +1 from me.

@benbjohnson -- would you mind re-reviewing?

[benbjohnson]

lgtm