The golang bindings currently use finalizers to manage native resources. From what I understand, this has some downsides mainly related to the fact that there aren’t strong guarantees around when finalizers will be run. It sounds like one particular issue that may arise is when there is a large quantity of native memory that needs to be freed but an absence of memory pressure in Go. This can result in garbage collection (and the finalizers) not being run, preventing the release of native resources.
The Java bindings have similar concerns, and they address them by providing methods to explicitly close native resources. I’m not an expert with Go, but I’m wondering if we should do the same thing there. Are there any Go users here that have opinions on this?
I don’t see anything wrong with constraining the lifetime of data read from FDB to the scope of the transaction.
This is how many Go KV-stores work. If you want the data outside of the transaction block to stay alive, you copy it. All pointers after commit or abort are invalid.
I don’t think an explicit function to free is that bad either, especially if it is scoped to a transaction and you can just defer it before you run the transaction.
BoltDB (used in etcd)
“Please note that values returned from Get() are only valid while the transaction is open. If you need to use a value outside of the transaction then you must use copy() to copy it to another byte slice.”
“Please note that values returned from Get() are only valid while the transaction is open. If you need to use a value outside of the transaction then you must use copy() to copy it to another byte slice.”
I’m willing to add this API to manually free resources if that’s acceptable to the team. I’d have to see how that interacts with backwards compatability of the public API, but I think it would work.
Note that the stdlib for Go frees associated resources in finalizers for some of its stuff (notable files and sockets), so it’s not necessarily a bad thing ™. It is true that it’s frowned upon to do work in Finalizers in Java, but that being said there is prior art here for KV stores in Java using Finalizers to do cleanup - the RocksDB Java bindings also do work in their Java finalizers.
However, both the Go stdlib and the RocksDB Java bindings immediately free associated resources when the file/socket/transaction is closed. When the underlying resources are freed a flag is set (or in Go’s case, the file is nilled out), and the finalizers will check to see if the resource has been freed before running to prevent double-freeing.
Rather than offer a separate API to free FDB native resources, I’d prefer copying this pattern in the Go/Java bindings by destroying the underlying transaction object when it’s canceled/completed. Although, I have not seen any memory pressure issues when running the FDB Go bindings though, so I wouldn’t rank this as important for me.
If I understand correctly, I think this is what the Java bindings are doing now. Objects holding native resources implement AutoCloseable and will release their resources when they are closed. If close is not called, then the finalizer cleans up the resources. I think we’ve technically deprecated the behavior in the finalizers because it has an associated cost and relying on it can lead to trouble, but for now it exists.
From what I can tell, we’d be subject to similar problems in the Go bindings, although it may require a specific workload to encounter them. My thought was that the Go API would look similar to what Java is doing now, possibly also deprecating and eventually removing the finalizer behavior.
So looks like people have had a similar question here: Redirecting to Google Groups. Ian’s suggestion was pretty sane:
I would say that using both defer and a finalizer is overkill. Pick one or the other. You either need to remember to free resources or you don’t. I don’t see an advantage to handling it both ways.
Your plan to migrate sounds make it explicit sounds sane. I’m happy with either, personally, since the FDB Database.Transaction method sounds like it would handle the cleanup for me anyway, and I’ve not seen any memory pressure issues with the current FDB Go bindings.
I think it would be great if you wanted to work on this. We should write up what we want this to look like in a GitHub issue and then we could assign it to you.