errose28 commented on PR #6482: URL: https://github.com/apache/ozone/pull/6482#issuecomment-2076110548
> >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 > ... That is the entire point of this change. This comes back to [this discussion](https://github.com/apache/ozone/pull/6482#discussion_r1578655136). You are correct that this doesn't work for when there is an "inspect" between the read and write, but this doesn't happen in the one example provided by the document. It seems the document is missing a section demonstrating "the entire point of the change". > 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. This is a great point. I also had only thought about failing early in the context of create key, not on each block operation. Storing the expected ID on the server makes the check on each block boundary possible. Let's add it to the doc. > 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. In this context I was trying to look at approaches from a top down API level, as in what does the client see and is it clear what is happening. While the conventions you mention are here are important to consider too, they are internal details that we have already discussed. It seems there has not been much discussion on what things look like to the client which was the point of this comment. The reason that the third approach looks strange from the client's perspective is that if you visualize generation ID as a sort of optimistic lock, it looks like the lock is released, and thus loses its guarantees, when create is called. For example: ``` info, genID = getInfo(key) // lock "acquired" optimistically if info != expected: ostream = rewrite(key, genID) // To the casual reader, it looks like the lock is "released" here. write to ostream // Is this safe? Yes, even though it doesn't look like it. commit(key) // Is the lock from earlier still respected? Yes, but unintuitive from the API structure due to server "magic". ``` However, I think your idea for failing early on block allocations is solid and outweighs the odd looks of this API and wider spread proto changes. With some doc updates to outline this case as only possible when the ID is passed on create, I'm ok to go forward with this implementation. -- 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]
