Skip to content

Commit 9288056

Browse files
committed
nfs4: voluntarily offer delegation id there are multiple opens on the file
Motivation: in a access loop like: ``` for i in 1...event_count: open-> read ->close process (event) ``` server might detect the pattern and voluntarily offer a delegation, unless client explicitly rejects it. Modification: - Update FileTracker to use AdaptiveDelegationLogic to offer delegations. - Clean the AdaptiveDelegationLogic on client dispose. NOTE: For now, we set the use sizes to 4096, and idle timeout 120s. The best practice still to discover. Result: open-read-close loop detected and optimized Acked-by: Marina Sahakyan Target: master
1 parent 64902e4 commit 9288056

File tree

3 files changed

+78
-8
lines changed

3 files changed

+78
-8
lines changed

core/src/main/java/org/dcache/nfs/util/AdaptiveDelegationLogic.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,12 @@ public AdaptiveDelegationLogic(int maxActiveQueueSize, int maxEvictionQueueSize,
211211
private ClientQueues getClientQueues(NFS4Client client) {
212212
return clientQueuesMap.computeIfAbsent(
213213
client.getId(),
214-
k -> new ClientQueues(maxEvictionQueueSize, maxActiveQueueSize)
214+
k -> {
215+
client.addDisposeListener( (c) -> {
216+
clientQueuesMap.remove(c.getId());
217+
});
218+
return new ClientQueues(maxEvictionQueueSize, maxActiveQueueSize);
219+
}
215220
);
216221
}
217222

core/src/main/java/org/dcache/nfs/v4/FileTracker.java

Lines changed: 39 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import com.google.common.util.concurrent.Striped;
2323

2424
import java.io.IOException;
25+
import java.time.Duration;
2526
import java.util.ArrayList;
2627
import java.util.Collection;
2728
import java.util.Iterator;
@@ -38,6 +39,7 @@
3839
import org.dcache.nfs.status.InvalException;
3940
import org.dcache.nfs.status.ShareDeniedException;
4041
import org.dcache.nfs.status.StaleException;
42+
import org.dcache.nfs.util.AdaptiveDelegationLogic;
4143
import org.dcache.nfs.v4.xdr.nfs4_prot;
4244
import org.dcache.nfs.v4.xdr.nfs_fh4;
4345
import org.dcache.nfs.v4.xdr.open_delegation_type4;
@@ -72,6 +74,15 @@ public class FileTracker {
7274
*/
7375
private final Map<Opaque, List<DelegationState>> delegations = new ConcurrentHashMap<>();
7476

77+
/**
78+
* Heuristic to offer delegations.
79+
*
80+
* FIXME: for now we use a fixed sizes and timeout. THe best practice still should be identified.
81+
*/
82+
private final AdaptiveDelegationLogic adlHeuristic =
83+
new AdaptiveDelegationLogic(4096, 4096, Duration.ofSeconds(120));
84+
85+
7586
private static class OpenState {
7687

7788
private final NFS4Client client;
@@ -226,7 +237,13 @@ public record OpenRecord(stateid4 openStateId, stateid4 delegationStateId, boole
226237
*/
227238
public OpenRecord addOpen(NFS4Client client, StateOwner owner, Inode inode, int shareAccess, int shareDeny) throws ChimeraNFSException {
228239

240+
// client explicitly refused delegation
241+
boolean acceptsDelegation = (shareAccess & nfs4_prot.OPEN4_SHARE_ACCESS_WANT_NO_DELEG) == 0;
242+
243+
// client explicitly requested read delegation
229244
boolean wantReadDelegation = (shareAccess & nfs4_prot.OPEN4_SHARE_ACCESS_WANT_READ_DELEG) != 0;
245+
246+
// client explicitly requested write delegation
230247
boolean wantWriteDelegation = (shareAccess & nfs4_prot.OPEN4_SHARE_ACCESS_WANT_WRITE_DELEG) != 0;
231248

232249
Opaque fileId = new Opaque(inode.getFileId());
@@ -255,13 +272,17 @@ public OpenRecord addOpen(NFS4Client client, StateOwner owner, Inode inode, int
255272

256273
/*
257274
* delegation is possible if:
275+
* - client has not explicitly requested no delegation
258276
* - client has a callback channel
259277
* - client does not have a delegation for this file
260278
* - no other open has write access
261279
*/
262-
boolean canDelegateRead = client.getCB() != null &&
263-
(existingDelegations == null || existingDelegations.stream().noneMatch(d -> d.client().getId() == client.getId())) &&
264-
opens.stream().noneMatch(os -> (os.shareAccess & nfs4_prot.OPEN4_SHARE_ACCESS_WRITE) != 0);
280+
boolean canDelegateRead = acceptsDelegation && (client.getCB() != null &&
281+
(existingDelegations == null ||
282+
existingDelegations.stream()
283+
.noneMatch(d -> d.client().getId() == client.getId())) &&
284+
opens.stream()
285+
.noneMatch(os -> (os.shareAccess & nfs4_prot.OPEN4_SHARE_ACCESS_WRITE) != 0));
265286

266287
// recall any read delegations if write
267288
if ((existingDelegations != null) && (shareAccess & nfs4_prot.OPEN4_SHARE_ACCESS_WRITE) != 0) {
@@ -290,8 +311,7 @@ public OpenRecord addOpen(NFS4Client client, StateOwner owner, Inode inode, int
290311
// access mode and return the same stateid as required by rfc5661#18.16.3
291312

292313
for (OpenState os : opens) {
293-
if (os.client.getId() == client.getId() &&
294-
os.getOwner().equals(owner)) {
314+
if (os.client.getId() == client.getId()) {
295315
os.shareAccess |= shareAccess;
296316
os.shareDeny |= shareDeny;
297317

@@ -305,6 +325,19 @@ public OpenRecord addOpen(NFS4Client client, StateOwner owner, Inode inode, int
305325
os.stateid.seqid++;
306326
//we need to return copy to avoid modification by concurrent opens
307327
var openStateid = new stateid4(os.stateid.other, os.stateid.seqid);
328+
329+
// yet another open from the same client. Let's check if we can delegate.
330+
if (canDelegateRead && (os.shareAccess & nfs4_prot.OPEN4_SHARE_ACCESS_BOTH) == nfs4_prot.OPEN4_SHARE_ACCESS_READ &&
331+
(wantReadDelegation || adlHeuristic.shouldDelegate(client, inode))) {
332+
333+
var delegationState = client.createDelegationState(os.getOwner());
334+
var delegation = new DelegationState(client, delegationState, open_delegation_type4.OPEN_DELEGATE_READ);
335+
delegations.computeIfAbsent(fileId, x -> new ArrayList<>(1))
336+
.add(delegation);
337+
338+
return new OpenRecord(openStateid, delegationState.stateid(), true);
339+
}
340+
308341
return new OpenRecord(openStateid, null, false);
309342
}
310343
}
@@ -320,8 +353,7 @@ public OpenRecord addOpen(NFS4Client client, StateOwner owner, Inode inode, int
320353
var openStateid = new stateid4(stateid.other, stateid.seqid);
321354

322355
// REVISIT: currently only read-delegations are supported
323-
if (wantReadDelegation && canDelegateRead) {
324-
// REVISIT: currently only read-delegations are supported
356+
if (canDelegateRead && (wantReadDelegation || adlHeuristic.shouldDelegate(client, inode))) {
325357
var delegationStateid = client.createDelegationState(state.getStateOwner());
326358
delegations.computeIfAbsent(fileId, x -> new ArrayList<>(1))
327359
.add(new DelegationState(client, delegationStateid, open_delegation_type4.OPEN_DELEGATE_READ));

core/src/test/java/org/dcache/nfs/v4/FileTrackerTest.java

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
import static org.dcache.nfs.v4.xdr.nfs4_prot.OPEN4_SHARE_ACCESS_READ;
3939
import static org.dcache.nfs.v4.xdr.nfs4_prot.OPEN4_SHARE_ACCESS_WRITE;
4040
import static org.dcache.nfs.v4.xdr.nfs4_prot.OPEN4_SHARE_ACCESS_BOTH;
41+
import static org.dcache.nfs.v4.xdr.nfs4_prot.OPEN4_SHARE_ACCESS_WANT_NO_DELEG;
4142
import static org.mockito.ArgumentMatchers.any;
4243
import static org.mockito.ArgumentMatchers.anyBoolean;
4344
import static org.mockito.Mockito.mock;
@@ -277,4 +278,36 @@ public void shouldAllowMultipleReadDelegation() throws Exception {
277278
assertTrue("Read delegation not granted", openRecord2.hasDelegation());
278279

279280
}
281+
282+
@Test
283+
public void shouldIssueReadDelegationOnMultipleOpens() throws Exception {
284+
285+
NFS4Client client = createClient(sh);
286+
StateOwner stateOwner = client.getOrCreateOwner("client".getBytes(StandardCharsets.UTF_8), new seqid4(0));
287+
288+
nfs_fh4 fh = generateFileHandle();
289+
Inode inode = Inode.forFile(fh.value);
290+
291+
var openRecord1 = tracker.addOpen(client, stateOwner, inode, OPEN4_SHARE_ACCESS_READ, 0);
292+
assertFalse("Delegation not expected, but granted", openRecord1.hasDelegation());
293+
294+
var openRecord2 = tracker.addOpen(client, stateOwner, inode, OPEN4_SHARE_ACCESS_READ, 0);
295+
assertTrue("Read opportunistic delegation not granted", openRecord2.hasDelegation());
296+
}
297+
298+
@Test
299+
public void shouldNotIssueReadDelegation() throws Exception {
300+
301+
NFS4Client client = createClient(sh);
302+
StateOwner stateOwner = client.getOrCreateOwner("client".getBytes(StandardCharsets.UTF_8), new seqid4(0));
303+
304+
nfs_fh4 fh = generateFileHandle();
305+
Inode inode = Inode.forFile(fh.value);
306+
307+
var openRecord1 = tracker.addOpen(client, stateOwner, inode, OPEN4_SHARE_ACCESS_READ | OPEN4_SHARE_ACCESS_WANT_NO_DELEG, 0);
308+
assertFalse("Unwanted delegation", openRecord1.hasDelegation());
309+
310+
var openRecord2 = tracker.addOpen(client, stateOwner, inode, OPEN4_SHARE_ACCESS_READ | OPEN4_SHARE_ACCESS_WANT_NO_DELEG, 0);
311+
assertFalse("Unwanted delegation", openRecord2.hasDelegation());
312+
}
280313
}

0 commit comments

Comments
 (0)