fixes some memory and concurrency bugs in gbucket

[stephenplaza]

This change is

[DocSavage]

Review status: 0 of 3 files reviewed at latest revision, 1 unresolved discussion.

---

datatype/imageblk/write.go, line 379 [r1] (raw file):
It looks like not all errors are logged on the PutCallback() so we should have a dvid.Errorf() or dvid.Criticalf() here.

---

Comments from the review on Reviewable.io

[stephenplaza]

Review status: 0 of 3 files reviewed at latest revision, 1 unresolved discussion.

---

datatype/imageblk/write.go, line 379 [r1] (raw file):
Looking at your old code, does this mean that a post can fail and an error message is produced but the actually calling function is unaware of failure? I don't see the error returned from PutChunk being used in the original code.

---

Comments from the review on Reviewable.io

[DocSavage]

Review status: 0 of 3 files reviewed at latest revision, 1 unresolved discussion.

---

datatype/imageblk/write.go, line 379 [r1] (raw file):
Yes. I'm not sure the best way to handle errors in asynchronous other than logging them. For example, if someone deletes a data instance, it spawns a goroutine to do the potentially long deletion but returns immediately with a 200 saying it's started the deletion. At that point, there is no more connection with the client. In the case of a POST subvolume, it is synchronous from the client so there should be an error channel instead of just a sync.WaitGroup. But currently, it's just treated like what you are doing here -- spawn a goroutine and log an error if there is one.

---

Comments from the review on Reviewable.io

[DocSavage]

Reviewed 2 of 3 files at r1, 1 of 1 files at r2.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

Comments from the review on Reviewable.io