search

weaviate / t2v-transformers-models

This is the repo for the container that holds the models for the text2vec-transformers module
BSD 3-Clause "New" or "Revised" License
39 stars 27 forks source link

increase 'timeout-keep-alive' from 5 to 60 #33

Closed StefanBogdan closed 1 year ago

bobvanluijt commented 1 year ago

@antas-marcin or @etiennedi – can you take a look at this please?

etiennedi commented 1 year ago

I need some context on what this does and why, please. Thanks.

bobvanluijt commented 1 year ago

@byronvoorbach – can you help with this in @StefanBogdan's absence?

StefanBogdan commented 1 year ago

Hi @etiennedi , this increases the Keep Alive time span for the Connections. Docs in here.

I have noticed a lot of connection reset by peer errors from Weaviate (returned per item by batch creation), and nothing in Weaviate logs, so I wanted to increase the connection keep alive timeout. If you think this is not needed please close this PR, without merging.

etiennedi commented 1 year ago

Thanks, that's helpful. If this solved a problem, I see no reason why not to merge it.

The reason I was initially confused is that - based on my understanding of TCP - keep-alive would only be needed to keep an idle connection open. So while an HTTP request is still open, keep alive should not play a role. It should only be a performance boost for reuse.

But if you're sure that this fixes the problem, I'm happy to merge this. I don't think it would hurt.

antas-marcin commented 1 year ago

@StefanBogdan do you think that this setting should be also applied to all other our inference containers?

StefanBogdan commented 1 year ago

This does not add any value, closing it.