Skip to main content
aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorXavier Coulon2015-12-07 10:33:25 +0000
committerJeff Johnston2015-12-07 22:52:34 +0000
commit05a1d3e072a121eca23f8bc64d11187065e711b2 (patch)
tree58608a9e19b081b7d02462c940d207663f3ff978 /containers/org.eclipse.linuxtools.docker.core/src/org/eclipse
parent7726582fa9334be4113f4f31f78986150672d114 (diff)
downloadorg.eclipse.linuxtools-05a1d3e072a121eca23f8bc64d11187065e711b2.tar.gz
org.eclipse.linuxtools-05a1d3e072a121eca23f8bc64d11187065e711b2.tar.xz
org.eclipse.linuxtools-05a1d3e072a121eca23f8bc64d11187065e711b2.zip
Bug 483769 - DockerExplorerView flickers when container info is
displayed and refreshed Added a map to track containers by id and re-use the previous container info when creating a new instance of DockerContainer for a given container, so that associated containerInfo does not need to be initialized again. Making sure that containers are sorted by name. Refactored the listContainers() method based on https://git.eclipse.org/r/#/c/54209/3 Change-Id: I46aed8ca720041d5d019f80eb75d0c2f78c5e0ea Signed-off-by: Xavier Coulon <xcoulon@redhat.com> Reviewed-on: https://git.eclipse.org/r/62082 Tested-by: Hudson CI Reviewed-by: Jeff Johnston <jjohnstn@redhat.com> (cherry picked from commit 87a3781d972e8c61056418d351745e9eb5616eae) Reviewed-on: https://git.eclipse.org/r/62155
Diffstat (limited to 'containers/org.eclipse.linuxtools.docker.core/src/org/eclipse')
-rw-r--r--containers/org.eclipse.linuxtools.docker.core/src/org/eclipse/linuxtools/internal/docker/core/DockerConnection.java135
-rw-r--r--containers/org.eclipse.linuxtools.docker.core/src/org/eclipse/linuxtools/internal/docker/core/DockerContainer.java32
2 files changed, 126 insertions, 41 deletions
diff --git a/containers/org.eclipse.linuxtools.docker.core/src/org/eclipse/linuxtools/internal/docker/core/DockerConnection.java b/containers/org.eclipse.linuxtools.docker.core/src/org/eclipse/linuxtools/internal/docker/core/DockerConnection.java
index 52ace62931..b266d2f248 100644
--- a/containers/org.eclipse.linuxtools.docker.core/src/org/eclipse/linuxtools/internal/docker/core/DockerConnection.java
+++ b/containers/org.eclipse.linuxtools.docker.core/src/org/eclipse/linuxtools/internal/docker/core/DockerConnection.java
@@ -18,7 +18,9 @@ import java.nio.ByteBuffer;
import java.nio.channels.WritableByteChannel;
import java.nio.file.FileSystems;
import java.util.ArrayList;
+import java.util.Collection;
import java.util.Collections;
+import java.util.Comparator;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
@@ -163,7 +165,10 @@ public class DockerConnection implements IDockerConnection, Closeable {
// private Set<String> printIds = new HashSet<String>();
+ // containers sorted by name
private List<IDockerContainer> containers;
+ // containers indexed by id
+ private Map<String, IDockerContainer> containersById;
private boolean containersLoaded = false;
private List<IDockerImage> images;
private boolean imagesLoaded = false;
@@ -357,6 +362,9 @@ public class DockerConnection implements IDockerConnection, Closeable {
}
}
+ // TODO: we might need something more fine grained, to indicate which
+ // container changed, was added or was removed, so we can refresh the UI
+ // accordingly.
public void notifyContainerListeners(List<IDockerContainer> list) {
if (containerListeners != null) {
Object[] listeners = containerListeners.getListeners();
@@ -400,23 +408,14 @@ public class DockerConnection implements IDockerConnection, Closeable {
@Override
public List<IDockerContainer> getContainers(final boolean force) {
- List<IDockerContainer> latestContainers;
- synchronized (containerLock) {
- latestContainers = this.containers;
- }
if (!isContainersLoaded() || force) {
try {
- latestContainers = listContainers();
+ return listContainers();
} catch (DockerException e) {
- synchronized (containerLock) {
- this.containers = Collections.emptyList();
- }
Activator.log(e);
- } finally {
- this.containersLoaded = true;
}
}
- return latestContainers;
+ return this.containers;
}
@Override
@@ -506,49 +505,105 @@ public class DockerConnection implements IDockerConnection, Closeable {
}
private List<IDockerContainer> listContainers() throws DockerException {
- final List<IDockerContainer> dclist = new ArrayList<>();
- synchronized (containerLock) {
- List<Container> list = null;
- try {
- synchronized (clientLock) {
- // Check that client is not null as this connection may have
- // been closed but there is an async request to update the
- // containers list left in the queue
- if (client == null)
- return dclist;
- list = client.listContainers(
- DockerClient.ListContainersParam.allContainers());
+ final Map<String, IDockerContainer> updatedContainers = new HashMap<>();
+ try {
+ final List<Container> nativeContainers = new ArrayList<>();
+ synchronized (clientLock) {
+ // Check that client is not null as this connection may have
+ // been closed but there is an async request to update the
+ // containers list left in the queue
+ if (client == null) {
+ // in that case the list becomes empty, which is fine is
+ // there's no client.
+ return Collections.emptyList();
}
- } catch (com.spotify.docker.client.DockerException
- | InterruptedException e) {
- throw new DockerException(
- NLS.bind(
- Messages.List_Docker_Containers_Failure,
- this.getName()), e);
+ nativeContainers.addAll(client.listContainers(
+ DockerClient.ListContainersParam.allContainers()));
}
-
// We have a list of containers. Now, we translate them to our own
// core format in case we decide to change the underlying engine
// in the future.
- for (Container c : list) {
+ for (Container nativeContainer : nativeContainers) {
// For containers that have exited, make sure we aren't tracking
// them with a logging thread.
- if (c.status().startsWith(Messages.Exited_specifier)) {
- if (loggingThreads.containsKey(c.id())) {
- loggingThreads.get(c.id()).requestStop();
- loggingThreads.remove(c.id());
+ if (nativeContainer.status()
+ .startsWith(Messages.Exited_specifier)) {
+ synchronized (loggingThreads) {
+ if (loggingThreads.containsKey(nativeContainer.id())) {
+ loggingThreads.get(nativeContainer.id())
+ .requestStop();
+ loggingThreads.remove(nativeContainer.id());
+ }
}
}
- if (!c.status().equals(Messages.Removal_In_Progress_specifier)) {
- dclist.add(new DockerContainer(this, c));
+ // skip containers that are being removed
+ if (nativeContainer.status()
+ .equals(Messages.Removal_In_Progress_specifier)) {
+ continue;
+ }
+ // re-use info from existing container with same id
+ if (this.containers != null && this.containersById
+ .containsKey(nativeContainer.id())) {
+ final IDockerContainer container = this.containersById
+ .get(nativeContainer.id());
+ updatedContainers.put(nativeContainer.id(),
+ new DockerContainer(this, nativeContainer,
+ container.info()));
+ } else {
+ updatedContainers.put(nativeContainer.id(),
+ new DockerContainer(this, nativeContainer));
}
}
- containers = dclist;
+ } catch (com.spotify.docker.client.DockerException
+ | InterruptedException e) {
+ throw new DockerException(
+ NLS.bind(Messages.List_Docker_Containers_Failure,
+ this.getName()),
+ e);
+ } finally {
+ // assign the new list of containers in a locked block of code to
+ // prevent concurrent access, even if an exception was raised.
+ synchronized (containerLock) {
+ this.containersById = updatedContainers;
+ this.containers = sort(this.containersById.values(),
+ new Comparator<IDockerContainer>() {
+
+ @Override
+ public int compare(final IDockerContainer container,
+ final IDockerContainer otherContainer) {
+ return container.name()
+ .compareTo(otherContainer.name());
+ }
+
+ });
+
+ this.containersLoaded = true;
+ }
}
+
// perform notification outside of containerLock so we don't have a View
// causing a deadlock
- notifyContainerListeners(dclist);
- return dclist;
+ // TODO: we should probably notify the listeners only if the containers
+ // list changed.
+ notifyContainerListeners(this.containers);
+ return this.containers;
+ }
+
+ /**
+ * Sorts the given values using the given comparator and returns the result
+ * in a {@link List}
+ *
+ * @param values
+ * the values to sort
+ * @param comparator
+ * the comparator to use
+ * @return the list of sorted values
+ */
+ private <T> List<T> sort(final Collection<T> values,
+ final Comparator<T> comparator) {
+ final List<T> result = new ArrayList<>(values);
+ Collections.sort(result, comparator);
+ return result;
}
@Override
diff --git a/containers/org.eclipse.linuxtools.docker.core/src/org/eclipse/linuxtools/internal/docker/core/DockerContainer.java b/containers/org.eclipse.linuxtools.docker.core/src/org/eclipse/linuxtools/internal/docker/core/DockerContainer.java
index 48f2397b39..e62f220728 100644
--- a/containers/org.eclipse.linuxtools.docker.core/src/org/eclipse/linuxtools/internal/docker/core/DockerContainer.java
+++ b/containers/org.eclipse.linuxtools.docker.core/src/org/eclipse/linuxtools/internal/docker/core/DockerContainer.java
@@ -18,6 +18,7 @@ import org.eclipse.linuxtools.docker.core.IDockerContainer;
import org.eclipse.linuxtools.docker.core.IDockerContainerInfo;
import org.eclipse.linuxtools.docker.core.IDockerPortMapping;
+import com.spotify.docker.client.DockerClient;
import com.spotify.docker.client.messages.Container;
public class DockerContainer implements IDockerContainer {
@@ -34,8 +35,17 @@ public class DockerContainer implements IDockerContainer {
private Long sizeRootFs;
private IDockerContainerInfo containerInfo;
+ /**
+ * Constructor.
+ *
+ * @param connection
+ * the Docker connection
+ * @param container
+ * the underlying {@link Container} data returned by the
+ * {@link DockerClient}
+ */
public DockerContainer(final IDockerConnection connection,
- Container container) {
+ final Container container) {
this.parent = connection;
this.id = container.id();
this.image = container.image();
@@ -62,6 +72,26 @@ public class DockerContainer implements IDockerContainer {
// TODO: include volumes
}
+ /**
+ * Constructor.
+ *
+ * @param connection
+ * the Docker connection
+ * @param container
+ * the underlying {@link Container} data returned by the
+ * {@link DockerClient}
+ * @param containerInfo
+ * the {@link IDockerContainerInfo} that was previously retrieved
+ * for this {@link IDockerContainer}, assuming it did not change
+ * in the mean time.
+ */
+ public DockerContainer(final IDockerConnection connection,
+ final Container container,
+ final IDockerContainerInfo containerInfo) {
+ this(connection, container);
+ this.containerInfo = containerInfo;
+ }
+
@Override
public IDockerConnection getConnection() {
return parent;

Back to the top