-
Notifications
You must be signed in to change notification settings - Fork 9.2k
HDFS-16283. RBF: reducing the load of renewLease() RPC #4524
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
802a405
de5120f
2d6e01e
642f920
86ad891
6739c22
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -291,8 +291,10 @@ public HdfsFileStatus create(String src, FsPermission masked, | |
| RemoteLocation createLocation = null; | ||
| try { | ||
| createLocation = rpcServer.getCreateLocation(src, locations); | ||
| return rpcClient.invokeSingle(createLocation, method, | ||
| HdfsFileStatus status = rpcClient.invokeSingle(createLocation, method, | ||
| HdfsFileStatus.class); | ||
| status.setNamespace(createLocation.getNameserviceId()); | ||
| return status; | ||
| } catch (IOException ioe) { | ||
| final List<RemoteLocation> newLocations = checkFaultTolerantRetry( | ||
| method, src, ioe, createLocation, locations); | ||
|
|
@@ -377,8 +379,11 @@ public LastBlockWithStatus append(String src, final String clientName, | |
| RemoteMethod method = new RemoteMethod("append", | ||
| new Class<?>[] {String.class, String.class, EnumSetWritable.class}, | ||
| new RemoteParam(), clientName, flag); | ||
| return rpcClient.invokeSequential( | ||
| locations, method, LastBlockWithStatus.class, null); | ||
| RemoteResult result = rpcClient.invokeSequential( | ||
| method, locations, LastBlockWithStatus.class, null); | ||
| LastBlockWithStatus lbws = (LastBlockWithStatus) result.getResult(); | ||
| lbws.getFileStatus().setNamespace(result.getLocation().getNameserviceId()); | ||
| return lbws; | ||
| } | ||
|
|
||
| @Override | ||
|
|
@@ -759,14 +764,49 @@ public boolean mkdirs(String src, FsPermission masked, boolean createParent) | |
| } | ||
| } | ||
|
|
||
| private Map<String, FederationNamespaceInfo> getAvailableNamespaces() | ||
| throws IOException { | ||
| Map<String, FederationNamespaceInfo> allAvailableNamespaces = | ||
| new HashMap<>(); | ||
| namenodeResolver.getNamespaces().forEach( | ||
| k -> allAvailableNamespaces.put(k.getNameserviceId(), k)); | ||
| return allAvailableNamespaces; | ||
| } | ||
|
|
||
| /** | ||
| * Try to get a list of FederationNamespaceInfo for renewLease RPC. | ||
| */ | ||
| private List<FederationNamespaceInfo> getRenewLeaseNSs(List<String> namespaces) | ||
| throws IOException { | ||
| if (namespaces == null || namespaces.isEmpty()) { | ||
| return new ArrayList<>(namenodeResolver.getNamespaces()); | ||
| } | ||
| List<FederationNamespaceInfo> result = new ArrayList<>(); | ||
| Map<String, FederationNamespaceInfo> allAvailableNamespaces = | ||
| getAvailableNamespaces(); | ||
| for (String namespace : namespaces) { | ||
| if (!allAvailableNamespaces.containsKey(namespace)) { | ||
| return new ArrayList<>(namenodeResolver.getNamespaces()); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should use
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Of course,
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Get it. make sense to me. |
||
| } else { | ||
| result.add(allAvailableNamespaces.get(namespace)); | ||
| } | ||
| } | ||
| return result; | ||
| } | ||
|
|
||
| @Override | ||
| public void renewLease(String clientName) throws IOException { | ||
| public void renewLease(String clientName, List<String> namespaces) | ||
| throws IOException { | ||
| rpcServer.checkOperation(NameNode.OperationCategory.WRITE); | ||
|
|
||
| RemoteMethod method = new RemoteMethod("renewLease", | ||
| new Class<?>[] {String.class}, clientName); | ||
| Set<FederationNamespaceInfo> nss = namenodeResolver.getNamespaces(); | ||
| rpcClient.invokeConcurrent(nss, method, false, false); | ||
| new Class<?>[] {String.class, List.class}, clientName, null); | ||
| List<FederationNamespaceInfo> nss = getRenewLeaseNSs(namespaces); | ||
| if (nss.size() == 1) { | ||
| rpcClient.invokeSingle(nss.get(0).getNameserviceId(), method); | ||
|
Comment on lines
+805
to
+806
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nsId is getting passed from the client, if we get an array or so, you can figure out initially itself whether you have only one entry or not. so you can get rid of |
||
| } else { | ||
| rpcClient.invokeConcurrent(nss, method, false, false); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should have some caching here:
Like:
Initially initialise
availableNamespaceand for every call check from this, if some entry isn't found in the stored/cachedavailableNamespace, In that case callgetAvailableNamespaces()and update the value ofavailableNamespace,if still we don't find the entry after then we can return all the namespace what we are doing now