sodonnel commented on PR #6482: URL: https://github.com/apache/ozone/pull/6482#issuecomment-2075163977
> Server manages the update ID This does not work in the general case, where a client reads a key, inspects it and decides that it needs rewritten. The key on the server could have changed in the meantime resulting in lost updates. The client must pass the generation it expects to overwrite based on what it has read. It cannot just trust that whatever is currently on the server has not changed. That is the entire point of this change. > Client manages the update ID (currently proposal 2 in the doc): > > * The key create request would return an outputstream to the client the same as before. > * The client gets the update ID of the key to overwrite. > * The client reads and writes the data to rewrite the same as before. This is doable, but not quite as you described. The client would need to read the existing key first to get its meta data. Then, ideally it passes the generation on key open so it can fail fast. If the key has already changed, there is little point in going ahead and writing it all out only to fail at the end. An addition which I had not yet considered, is that even on block allocation the generation could be checked against that which is in the key table, so for a large object it could be check at each block boundary too. I have not looked at the block allocation code, but I think it must persist the allocated blocks in the open key table along with the key to allow for them to be garbage collected later if the client should crash. I am also not sure what the block allocation protocol looks like, but by storing the expectedGeneration on the server, we avoid any changes to the block allocation protocol and gain this feature. > To me, either of the first two options where only one side is responsible for storing the update ID as the write is ongoing make sense. The third option is a mashup of the others, which IMO is the least intuitive option. But the third option, is how things currently work for the other metadata fields in a key. To do differently is less intuitive as now this solution goes against how all the other fields are stored. To give an analogy from web-development - the current structure is to have the session store on the server, rather than in a cookie. What you want, is to split this new area into something like a cookie session which we also have the server side session. You have already cited that you don't like the current approach, but we are not going to change that. Sometimes it is better to stick with the conventions already in place, rather than going in a new direction that is possibly better, but possibly not. In my opinion, both have their pros and cons and there is no clear best answer. The HSync code has added information to the openKey table. It has added it to the MetaData map, so it has avoided adding an extra protobuf field in the protobuf at all. That is also something I could consider, but it will be less efficient and kind of sidesteps a lot of the static type checking Java can do for us, so bugs are easier to get in. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
