I prefer not to introduce the ref counting - it is cryptic and if it is known to need replacement, these are signs that it should not go in.
What is the exact affect treating shutdown the same as in couchdb store for now? (Meaning, shutdown is not really handled explicitly) Other than possibly complicating test scenarios, it hasn’t been clear that this is a problem (other than good citizenship dictating that you should gracefully close resources) I’m also still wondering if “sharing the client” is required - what are the affects of having multiple clients that connect to the same instance? These clients will only operate on different collections within that instance right? These would connect to the same endpoint, sure, but is that a problem other than inflating the connection pool slightly? It might make things more consistent with other artifact stores, with the sacrifice of some azure purity that not have real impact. If there is significant problem with not revising the SPI to better handle shutdown (or other aspects), an alternate approach would be to address the SPI first. I’m interested to know what others think. Thanks Tyson > On Jun 21, 2018, at 3:50 AM, Chetan Mehrotra <chetan.mehro...@gmail.com> > wrote: > > ArtifactStore SPI exposes a shutdown method which is responsible for > closing any resource owned by store implementation. > > Ccurrently for CouchDbRestStore it only shuts down ActorMaterializer which > is created one per instance. It does not shutdown Http pool which is shared > across 3 store instance. This is documented in PoolingRestClient. > > Now with CosmosDBArtifactStore we need to share a `DocumentClient` instance > which owns the underlying Netty connection pool. As all the store instance > talk to same db it makes sense to share the instance. > > However this sharing poses problem with shutdown. To handle that CosmosDB > PR introduces a `CountedReference` [1] which keeps an open/close count and > only closes when all references are closed. > > Wanted to check with team if that would be fine approach to take? > > Currently its bit tricky to manage components lifecycle and often we need > to rely on shutdown hooks to close the resources properly. May be we > revisit SPI/component lifecycle handling later and then review this > shutdown method handling > > Chetan Mehrotra > [1] > https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fincubator-openwhisk%2Fpull%2F3562%2Ffiles%23diff-&data=02%7C01%7Ctnorris%40adobe.com%7Cfbbbb0ade8ac4925e92b08d5d764dd9c%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636651750626753531&sdata=BJVvsBCyWCnzspYnYDyXrBfiLNk97vBJyJuDQzslcag%3D&reserved=0 > 9d57de71410575fd70240ac974be407d
