elixir-ecto / db_connection

Database connection behaviour
http://hexdocs.pm/db_connection/DBConnection.html
306 stars 113 forks source link

add hibernate option for long lived process #101

Closed redink closed 6 years ago

redink commented 6 years ago

could save memory as far as possible

redink commented 6 years ago

When I using hibernate for the connection process and query mongo continuously the memory of process just like:

------- {memory,3408}
------- {memory,3408}
------- {memory,3408}
------- {memory,3408}
------- {memory,3408}
------- {memory,3408}
------- {memory,3408}
------- {memory,3408}
------- {memory,3408}
------- {memory,3408}
------- {memory,3408}
------- {memory,3408}
------- {memory,3408}
------- {memory,6360}
------- {memory,3408}
------- {memory,3408}
------- {memory,3408}
------- {memory,3408}
------- {memory,3408}
------- {memory,3408}
------- {memory,3408}
------- {memory,3408}
------- {memory,3408}
------- {memory,3408}
------- {memory,3408}
------- {memory,3408}
------- {memory,3408}
------- {memory,3408}
------- {memory,3408}

but, after disabled the hibernate option, the memory of the long lived process:

------- {memory,16936}
------- {memory,16936}
------- {memory,16936}
------- {memory,17096}
------- {memory,17096}
------- {memory,17440}
------- {memory,21984}
------- {memory,21984}
------- {memory,21984}
------- {memory,21984}
------- {memory,21984}
------- {memory,21984}
------- {memory,21984}
------- {memory,22328}
------- {memory,29888}
------- {memory,29888}
------- {memory,29888}
------- {memory,29888}
------- {memory,29888}
------- {memory,29888}
------- {memory,29888}
------- {memory,29888}
------- {memory,29888}
------- {memory,29888}
------- {memory,29888}
------- {memory,29888}
------- {memory,29888}
------- {memory,14080}
------- {memory,14080}
------- {memory,14080}
------- {memory,14080}
------- {memory,14080}
------- {memory,17096}
------- {memory,17096}
------- {memory,17096}
------- {memory,17096}
------- {memory,17440}
------- {memory,21984}
------- {memory,21984}
------- {memory,21984}
------- {memory,21984}

Maybe hibernate is not friendly to CPU, but user could choose if enable the hibernate. So, I think it is necessary to add hibernate option. Please review !

josevalim commented 6 years ago

Thanks @redink! @fishcakez would be the best person to decide if we should merge this but I believe we would need to at least have tests. I am not sure if we can assert externally that a process is in hibernation but we need to assert that at least the code doesn't break if the hibernate flag is used.

redink commented 6 years ago

Thanks @josevalim ! As just we know, db_connection depends on https://hex.pm/packages/connection, and connection is one wrapper of gen_server. hibernation is one property for Erlang process and user can utilize the property via hibernate in gen_server. Fortunately, connection keeps hibernation in it and db_connection can use hibernation conveniently.

There are some example which using hibernate:

rabbitmq-server using gen_server2 in its source code and using hibernate in many modules, just like: https://github.com/rabbitmq/rabbitmq-server/blob/master/src/rabbit_channel.erl#L419

In ejabberd source code, its using gen_server and hibernate timeout: https://github.com/processone/ejabberd/blob/master/src/ejabberd_receiver.erl#L193

And in one early article https://www.metabrew.com/article/a-million-user-comet-application-with-mochiweb-part-2. The tester using hibernate.

Judicious use of hibernate means the mochiweb application memory levels out at 78MB Resident with 10k connections, much better than the 450MB we saw in Part 1. There was no significant increase in CPU usage.

fishcakez commented 6 years ago

I think that if we want to add a hibernate option it should follower a slightly smarter strategy rather than always hibernating. For example if we have a checkout or a checkin we know that its very likely that we will get another message almost immediately so hibernating is likely to impact performance there and not be beneficial.

My intuition is that we should see the same benefits if we only hibernate in the following cases:

@redink could you try this approach and report back? Also could you describe the methodology you used both this time and previously? For example if just start the processes and do nothing, or you are running a small number of queries. DBConnection.Connection processes have a different gc pattern to the rabbitmq, ejabberd and mochiweb examples because the vast majority of our garbage is generated in another process's heap. Its very possible we are being too clever here. Also a well written driver won't hold onto refc binaries because they should never be passed from another process so we shouldn't hit that.

redink commented 6 years ago

@fishcakez hmmmm, can't agree with you more. Before, I usually use hibernate timeout, just like the new feature hibernate_after in Erlang 20.x But, there is idle_timeout in db_connection, so it not easy to use hibernate timeout, these two timeout is conflicted. However, I push one new branch redink:smarter_hibernate, and create a new repo to show how I test them.

https://github.com/redink/db_connection_hibernate

The data shows that smarter hibernate also could save memory.

redink commented 6 years ago

The process that I trace is:

image

redink commented 6 years ago

@fishcakez ping ...

fishcakez commented 6 years ago

@redink sorry it'll be a couple of days until I can take a better look at this.

michalmuskala commented 6 years ago

If the problem is memory, maybe a better solution would be to manually trigger GC with :erlang.garbage_collect() rather than constantly hibernating the process. Wouldn't this be a bit less invasive while providing similar benefits?

redink commented 6 years ago

Maybe :erlang.garbage_collect() will reduce the memory leak and fragment, but it still has some problems:

And hibernate is a feature supported by Erlang official, I don't think it not better than :erlang.garbage_collect(). I also don't think it invasive, just because Erlang support hibernate_after option when start one gen_server process on 20.x (http://erlang.org/doc/man/gen_server.html#start_link-3).

redink commented 6 years ago

any update ?

fishcakez commented 6 years ago

@redink can you send a PR with your smarter branch with 2 changes:

fishcakez commented 6 years ago

Closing in favor of #103.