Commit 44fa34cd authored by Tigran Mkrtchyan's avatar Tigran Mkrtchyan
Browse files

nfs4: improve state handler concurrency

Motivation:
The state handler keeps track of NFSv4 clients. This duty is not
symmetric - updates happen when a client is added or removed, but
queried almost for any compound operation in a session. Thus on a
busy system we can observe thread serialization.

Modification:
replace synchronized blocks with read/write locks.

Result:
a better concurrency in a multi-client environment.

Acked-by: Paul Millar
Acked-by: Albert Rossi
Target: master
parent ea6f71a3
Pipeline #1948 passed with stage
in 1 minute and 9 seconds
/* /*
* Copyright (c) 2009 - 2020 Deutsches Elektronen-Synchroton, * Copyright (c) 2009 - 2021 Deutsches Elektronen-Synchroton,
* Member of the Helmholtz Association, (DESY), HAMBURG, GERMANY * Member of the Helmholtz Association, (DESY), HAMBURG, GERMANY
* *
* This library is free software; you can redistribute it and/or modify * This library is free software; you can redistribute it and/or modify
...@@ -32,6 +32,8 @@ import java.util.concurrent.TimeUnit; ...@@ -32,6 +32,8 @@ import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.Executors; import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantReadWriteLock;
import java.util.stream.Collectors; import java.util.stream.Collectors;
import org.dcache.nfs.ChimeraNFSException; import org.dcache.nfs.ChimeraNFSException;
...@@ -47,6 +49,8 @@ import org.dcache.oncrpc4j.util.Bytes; ...@@ -47,6 +49,8 @@ import org.dcache.oncrpc4j.util.Bytes;
import org.dcache.nfs.util.Cache; import org.dcache.nfs.util.Cache;
import org.dcache.nfs.util.NopCacheEventListener; import org.dcache.nfs.util.NopCacheEventListener;
import javax.annotation.concurrent.GuardedBy;
import static com.google.common.base.Preconditions.checkState; import static com.google.common.base.Preconditions.checkState;
public class NFSv4StateHandler { public class NFSv4StateHandler {
...@@ -63,9 +67,19 @@ public class NFSv4StateHandler { ...@@ -63,9 +67,19 @@ public class NFSv4StateHandler {
*/ */
private final AtomicInteger _clientId = new AtomicInteger(0); private final AtomicInteger _clientId = new AtomicInteger(0);
// mapping between server generated clietid and nfs_client_id, not confirmed yet /**
* Mapping between server generated clietid and nfs_client_id, not confirmed yet.
*/
private final ClientCache _clientsByServerId; private final ClientCache _clientsByServerId;
/**
* Read/write lock that guards the access to {@link #_clientsByServerId}.
*/
private final ReentrantReadWriteLock _accessLock = new ReentrantReadWriteLock();
private final Lock _readLock = _accessLock.readLock();
private final Lock _writeLock = _accessLock.writeLock();
/** /**
* Client's lease expiration time in seconds. * Client's lease expiration time in seconds.
*/ */
...@@ -129,19 +143,27 @@ public class NFSv4StateHandler { ...@@ -129,19 +143,27 @@ public class NFSv4StateHandler {
public void removeClient(NFS4Client client) { public void removeClient(NFS4Client client) {
synchronized (this) { _writeLock.lock();
try {
checkState(_running, "NFS state handler not running"); checkState(_running, "NFS state handler not running");
_clientsByServerId.remove(client.getId()); _clientsByServerId.remove(client.getId());
clientStore.removeClient(client.getOwnerId()); clientStore.removeClient(client.getOwnerId());
} finally {
_writeLock.unlock();
} }
client.tryDispose(); client.tryDispose();
} }
private synchronized void addClient(NFS4Client newClient) { private void addClient(NFS4Client newClient) {
checkState(_running, "NFS state handler not running"); _writeLock.lock();
_clientsByServerId.put(newClient.getId(), newClient); try {
clientStore.addClient(newClient.getOwnerId()); checkState(_running, "NFS state handler not running");
_clientsByServerId.put(newClient.getId(), newClient);
clientStore.addClient(newClient.getOwnerId());
} finally {
_writeLock.unlock();
}
} }
/** /**
...@@ -152,14 +174,12 @@ public class NFSv4StateHandler { ...@@ -152,14 +174,12 @@ public class NFSv4StateHandler {
* @throws StaleClientidException if there are no corresponding verified * @throws StaleClientidException if there are no corresponding verified
* valid record exist. * valid record exist.
*/ */
public synchronized NFS4Client getConfirmedClient(clientid4 clientid) throws StaleClientidException { public NFS4Client getConfirmedClient(clientid4 clientid) throws StaleClientidException {
NFS4Client client = getValidClient(clientid); NFS4Client client = getValidClient(clientid);
if (!client.isConfirmed()) { if (!client.isConfirmed()) {
throw new StaleClientidException("client not confirmed."); throw new StaleClientidException("client not confirmed.");
} }
return client; return client;
} }
...@@ -172,14 +192,12 @@ public class NFSv4StateHandler { ...@@ -172,14 +192,12 @@ public class NFSv4StateHandler {
* @throws StaleClientidException if there are no corresponding verified * @throws StaleClientidException if there are no corresponding verified
* valid record exist. * valid record exist.
*/ */
public synchronized NFS4Client getValidClient(clientid4 clientid) throws StaleClientidException { public NFS4Client getValidClient(clientid4 clientid) throws StaleClientidException {
NFS4Client client = getClient(clientid); NFS4Client client = getClient(clientid);
if (!client.isLeaseValid()) { if (!client.isLeaseValid()) {
throw new StaleClientidException("client expired."); throw new StaleClientidException("client expired.");
} }
return client; return client;
} }
...@@ -191,37 +209,53 @@ public class NFSv4StateHandler { ...@@ -191,37 +209,53 @@ public class NFSv4StateHandler {
* @return nfs client associated with clientid. * @return nfs client associated with clientid.
* @throws StaleClientidException if there are no corresponding record exist. * @throws StaleClientidException if there are no corresponding record exist.
*/ */
public synchronized NFS4Client getClient(clientid4 clientid) throws StaleClientidException { public NFS4Client getClient(clientid4 clientid) throws StaleClientidException {
checkState(_running, "NFS state handler not running"); _readLock.lock();
try {
checkState(_running, "NFS state handler not running");
NFS4Client client = _clientsByServerId.get(clientid); NFS4Client client = _clientsByServerId.get(clientid);
if(client == null) { if (client == null) {
throw new StaleClientidException("bad client id."); throw new StaleClientidException("bad client id.");
}
return client;
} finally {
_readLock.unlock();
} }
return client;
} }
public synchronized NFS4Client getClientIdByStateId(stateid4 stateId) throws ChimeraNFSException { public NFS4Client getClientIdByStateId(stateid4 stateId) throws ChimeraNFSException {
checkState(_running, "NFS state handler not running"); _readLock.lock();
try {
checkState(_running, "NFS state handler not running");
clientid4 clientId = new clientid4(Bytes.getLong(stateId.other, 0)); clientid4 clientId = new clientid4(Bytes.getLong(stateId.other, 0));
NFS4Client client = _clientsByServerId.get(clientId); NFS4Client client = _clientsByServerId.get(clientId);
if (client == null) { if (client == null) {
throw new BadStateidException("no client for stateid: " + stateId); throw new BadStateidException("no client for stateid: " + stateId);
}
return client;
} finally {
_readLock.unlock();
} }
return client;
} }
public synchronized NFS4Client getClient(sessionid4 id) throws ChimeraNFSException { public NFS4Client getClient(sessionid4 id) throws ChimeraNFSException {
checkState(_running, "NFS state handler not running");
clientid4 clientId = new clientid4(Bytes.getLong(id.value, 0)); _readLock.lock();
NFS4Client client = _clientsByServerId.get(clientId); try {
if (client == null) { checkState(_running, "NFS state handler not running");
throw new BadSessionException("session not found: " + id); clientid4 clientId = new clientid4(Bytes.getLong(id.value, 0));
NFS4Client client = _clientsByServerId.get(clientId);
if (client == null) {
throw new BadSessionException("session not found: " + id);
}
return client;
} finally {
_readLock.unlock();
} }
return client;
} }
/** /**
...@@ -230,12 +264,18 @@ public class NFSv4StateHandler { ...@@ -230,12 +264,18 @@ public class NFSv4StateHandler {
* *
* @return an existing client record or null, if not matching record found. * @return an existing client record or null, if not matching record found.
*/ */
public synchronized NFS4Client clientByOwner(byte[] ownerid) { public NFS4Client clientByOwner(byte[] ownerid) {
return _clientsByServerId
.stream() _readLock.lock();
.filter(c -> Arrays.equals(c.getOwnerId(), ownerid)) try {
.findAny() return _clientsByServerId
.orElse(null); .stream()
.filter(c -> Arrays.equals(c.getOwnerId(), ownerid))
.findAny()
.orElse(null);
} finally {
_readLock.unlock();
}
} }
public void updateClientLeaseTime(stateid4 stateid) throws ChimeraNFSException { public void updateClientLeaseTime(stateid4 stateid) throws ChimeraNFSException {
...@@ -252,10 +292,16 @@ public class NFSv4StateHandler { ...@@ -252,10 +292,16 @@ public class NFSv4StateHandler {
client.updateLeaseTime(); client.updateLeaseTime();
} }
public synchronized List<NFS4Client> getClients() { public List<NFS4Client> getClients() {
checkState(_running, "NFS state handler not running");
return _clientsByServerId.peek() _readLock.lock();
.collect(Collectors.toList()); try {
checkState(_running, "NFS state handler not running");
return _clientsByServerId.peek()
.collect(Collectors.toList());
} finally {
_readLock.unlock();
}
} }
public NFS4Client createClient(InetSocketAddress clientAddress, InetSocketAddress localAddress, int minorVersion, public NFS4Client createClient(InetSocketAddress clientAddress, InetSocketAddress localAddress, int minorVersion,
...@@ -315,7 +361,8 @@ public class NFSv4StateHandler { ...@@ -315,7 +361,8 @@ public class NFSv4StateHandler {
clientStore.wantReclaim(owner); clientStore.wantReclaim(owner);
} }
private synchronized void drainClients() { @GuardedBy("_writeLock")
private void drainClients() {
_clientsByServerId.stream() _clientsByServerId.stream()
.forEach(c -> { .forEach(c -> {
c.tryDispose(); c.tryDispose();
...@@ -326,12 +373,18 @@ public class NFSv4StateHandler { ...@@ -326,12 +373,18 @@ public class NFSv4StateHandler {
/** /**
* Shutdown session lease time watchdog thread. * Shutdown session lease time watchdog thread.
*/ */
public synchronized void shutdown() throws IOException { public void shutdown() throws IOException {
checkState(_running, "NFS state handler not running");
_running = false; _writeLock.lock();
drainClients(); try {
_cleanerScheduler.shutdown(); checkState(_running, "NFS state handler not running");
clientStore.close(); _running = false;
drainClients();
_cleanerScheduler.shutdown();
clientStore.close();
} finally {
_writeLock.unlock();
}
} }
/** /**
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment