zhengchenyu commented on code in PR #4756:
URL: https://github.com/apache/hadoop/pull/4756#discussion_r950658450
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java:
##########
@@ -3470,6 +3471,16 @@ HdfsFileStatus getFileInfo(final String src, boolean
resolveLink,
logAuditEvent(false, operationName, src);
throw e;
}
+ if (needLocation && haEnabled && haContext != null &&
+ haContext.getState().getServiceState() == OBSERVER &&
+ stat instanceof HdfsLocatedFileStatus) {
+ LocatedBlocks lbs = ((HdfsLocatedFileStatus) stat).getLocatedBlocks();
+ for (LocatedBlock b : lbs.getLocatedBlocks()) {
+ if (b.getLocations() == null || b.getLocations().length == 0) {
+ throw new ObserverRetryOnActiveException("Zero blocklocations for "
+ src);
+ }
+ }
+ }
Review Comment:
> Can we pull this into a common method like:
>
> ```java
> private void checkBlockLocationsIfObserver(Iterator<LocatedBlocks>
blocksIter) throws ObserverRetryOnActive {
> if (haEnabled && haContext != null &&
haContext.getState().getServiceState() == OBSERVER) {
> ...
> }
> }
> ```
>
> or two methods like
>
> ```java
> private boolean isObserver() { return haEnabled && haContext != null &&
haContext.getState().getServiceState() == OBSERVER; }
> private void checkBlockLocationsForObserver(LocatedBlocks blocks) throws
ObserverRetryOnActive { ... }
> ```
>
> Point being that we have 3 places with almost identical logic here, we
should try to consolidate.
good idea!
--
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]
