Incubaid / arakoon

A consistent distributed key-value store
http://arakoon.org
Apache License 2.0
84 stars 17 forks source link

Use a temporary file when receiving `head.db` #439

Closed NicolasT closed 10 years ago

NicolasT commented 10 years ago

During catchup, head.db was overwritten in-place, which is rather unsafe at several levels.

This commit changes the receiving mechanism to use a temporary file, and safely rename the received file (if successful) to the target head.db.

NicolasT commented 10 years ago

Didn't pass through CI yet, launching now.

NicolasT commented 10 years ago

arakoon-git-launcher 315

NicolasT commented 10 years ago

CI passed, all blue.

NicolasT commented 10 years ago

The question whether this approach is required was raised, due to the increased storage requirements in the head_dir. A different approach was proposed, which boiled down to basically removing head.db as well as all tlogs on a system whenever the transfer of a new head.db from another node is required during catchup (which implies all tlogs are too old anyway).

Whilst I understand this might be a concern, I am convinced an Arakoon node should at no time remove any information it contains, even though this information is very outdated and available on (a quorum of) other systems in the cluster. As such I think the approach implemented by this patch is the correct one.

mark-christiaens commented 10 years ago

I vote for Nicolas solution to go in. It's safer than the existing code and the most defensive solution. If anyone is convinced that we should go further then we can do another pull request.

domsj commented 10 years ago

Ok lgtm, merge it in