search

globalsign / mgo

The MongoDB driver for Go
Other
1.97k stars 230 forks source link

enable shrink the socket pool size #116

Closed gnawux closed 6 years ago

gnawux commented 6 years ago

This is an updated version of #87, changes:

Below is the original introduction

we found the mgo will allocate the pool size during burst traffic but won't close the sockets any more until restart the client or server.

And the mongo document defines two related query options

By implementing these two options, it could shrink the pool to minPoolSize after the sockets introduced by burst traffic timeout.

The idea comes from https://github.com/JodeZer/mgo , he investigated this issue and provide the initial commits.

I found there are still some issue in sockets maintenance, and had a PR against his repo JodeZer/mgo#1 .

This commit include JodeZer's commits and my fix, and I simplified the data structure. What's in this commit could be described as this figure:

+------------------------+
|        Session         | <-------+ Add options here
+------------------------+

+------------------------+
|        Cluster         | <-------+ Add options here
+------------------------+

+------------------------+
|        Server          | <-------+*Add options here
|                        |          *add timestamp when recycle a socket  +---+
|          +-----------+ |    +---+ *periodically check the unused sockets    |
|          | shrinker  <------+          and reclaim the timeout sockets. +---+
|          +-----------+ |                                                    |
|                        |                                                    |
+------------------------+                                                    |
                                                                              |
+------------------------+                                                    |
|        Socket          | <-------+ Add a field for last used times+---------+
+------------------------+
gnawux commented 6 years ago

@szank @domodwyer I updated #87 based on your review and added tests here.

gnawux commented 6 years ago

@szank Thanks for your comments, updated.

gnawux commented 6 years ago

looks the travis failure is not related with this patch

KJTsanaktsidis commented 6 years ago

👍 nice work! LGTM.

domodwyer commented 6 years ago

Hi @gnawux

This looks great, I'll get the build to pass with a bit of retry fun and then we'll get this merged.

Thanks so much for taking the time to help!

Dom

gnawux commented 6 years ago

It's my pleasure, and as I mentioned in the commit message, the original investigation credits to @JodeZer, I just did some tests and improvement.