Add Optimistic Concurrency Control / If-Match support to Node Objects (or all DB objects)

[lamont-granquist]

On the server side I think this is as simple as adding the webmachine generate_etag and last_modified hooks. It looks like bookshelf already does this. If erchef did this for node objects then client code could be modified to supply an If-Match header with the Etag in the PUT request which would then get rejected by the server if the object had been updated and the tag changed.

[stevendanna]

We've talked about something like this in the past. I'm 👍 on the general idea.

[lamont-granquist]

Yeah, I have some questions about how the client could usefully leverage this, but it seems like the servers should fully support Etags as part of useful extensions to the HTTP protocol and then client-side we can figure out how they can be made useful.

[rafaltrojniak]

Hello @stevendanna and @lamont-granquist .
I would be happy to help here, if you just guide me where to start from.

I had done some Erlang development in the past, and quite much Ruby recently when using chef.

[marcparadise]

@rafaltrojniak I can probably help you get started here. There are a few pieces to be modified:

• erchef - erlang - generate the etag, and perform etag comparison . Also includes new common_test and eunit tests
• omnibus - ruby/chef - ensure that openresty allows etag to be passed through both ways (it may be no changes are needed here)
• oc-chef-pedant - ruby/rspec - new tests

All of these components are contained within the chef-server repository. Also available is an integrated development VM which should keep it relatively simple. If you're available on IRC, we can talk further this week?

[rafaltrojniak]

Hello @marcparadise , sorry for not responding - notification got missed between other ones.

I had grabbed the repo and tried starting vagrant. Will try to move forward in the week.

Is there any module graph/guideline so I can find proper erlang(oc_erchef) module to modify ? It looks like big codebase and I feel lost :/

[marcparadise]

No worries. Yeah, it's a bit of a maze when coming in fresh. I'd have to refresh my memory on where etags fit into the webmachine flow, but the components involved at minimum would be:

• apps/oc_chef_wm/src/chef_wm_named_node.erl - webmachine resource for GET and PUTting a node. It may be as simple as adding a generate_etag/1 callback that can hash a #chef_node.serialized_object, and letting webmachine If-Match header behavior handle the rest.
• apps/chef_objects/src/chef_node.erl
• include/chef_types.hrl (that's where #chef_node lives)

Another approach is to add support for If-Unmodified-Since: we already capture a last-modified time every time we update the node object. If we provide to the client via Last-Modified header and th client hands it back to us on PUT, that might make it even simpler.

[lamont-granquist]

Is there any common "base class" (you'll have to translate from OO speak into something that might make sense in a functional erlang world) where we could fix this for all objects and not just node objects? And I do think that while we're adding generate_etag/1 support that we should add last_modifed/1 support while we're at it.

[rafaltrojniak]

Hello,
Just got some learning and looked-over the code

• @marcparadise thanks for the directions, that hoped me a lot in figuring how things work here. Adding function to generate etag in chef_node sounds like wise idea, and using that in chef_wm_named_node in form of a guard should be easy. Just need to get use to Webmachine.
• Using #node_name.updated_at as a base for an etag sounds like enough (timestamp with microseconds precision). I haven't fond a place where the field is used, and I'm afraid that the precision won't survive persistence (might be declared in database as timestamp with second precision). If that happens, we either need to increase storage precision, or introduce new field for that 'clock'

I'll dig more into the code and I plan to :

• Create function in chef_node to generate etag. For now based on updated_at field, so that might be changed in future easily
• Expose information from the etag through the Etag HTTP header, and updated_at as Last-Modified HTTP header
• Try to create guards when If-Match or If-Modified-Since headers are sent.
• Surround above with eunit tests

@lamont-granquist With the node Proof Of Concept I'll try to extend that to other objects (like nodes, environments and so one). I'm just afraid that because of the splitting objects (different records/modules definition for them) that will lead to some duplication, or using of complicated callbacks. But we'll see.

[rafaltrojniak]

I had some troubles with firing up the Vagrant box and tried to work on some eunit tests on my box.
Unfortunatelly neither main project or chef_object app eunit tests fail. Any idea why ?

[ chef_objects]$ make eunit
==> chef_objects (get-deps)
==> chef_objects (compile)
src/chef_client.erl:none: undefined parse transform 'mixer'
Compiling src/chef_client.erl failed:
ERROR: compile failed while processing /.../chef-server/src/oc_erchef/apps/chef_objects: rebar_abort
Makefile:51: recipe for target 'compile' failed
make: *** [compile] Error 1