[GitHub] [hadoop] xkrogen commented on pull request #4872: HDFS-16764. ObserverNamenode should throw ObserverRetryOnActiveException instead of FileNotFoundException during processing of addBlock rpc

[GitBox <gi...@apache.org>]

xkrogen commented on PR #4872:
URL: https://github.com/apache/hadoop/pull/4872#issuecomment-1320520221

   Sorry for being late to the party here. The changes to `getAdditionalBlock` look fine to me -- we already check for `WRITE` operation later in the same method, this just moves the check a little earlier. No issues there.
   
   The changes to `getAdditionalDatanode`, which legitimately seems not to do any write modification to the namespace (e.g. no write lock is held), are less obviously okay. I think the real problem here comes from the mismatch: `FSNamesystem#getAdditionalDatanode` considers itself a read-op, but in `ClientProtocol`, it is not annotated as `@ReadOnly`, so it's considered like a write op. So we can _either_ change it to `OperationCategory.WRITE`, as proposed in this PR, _or_ we can mark it as `@ReadOnly(isCoordinated = true)`. Either one would solve the current problem. Marking it read-only is better from a perf/scalability perspective if it really can be safely served by an ObserverNode. Looking through `getAdditionalDatanode`, the only part I would be worried about is that we generate a new block token as part of the response. AFAICT from the handling of the keys used by `BlockTokenSecretManager`, the DataNodes will fetch/trust keys produced by all NNs, not just the active, so I thin
 k this is all good.
   
   So, unless anyone sees a reason why we _can't_ mark `getAdditionalDatanode` as `@ReadOnly`, I would propose to do that instead of switching its `OperationCategory` to `WRITE`.


-- 
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: common-issues-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org