Skip to content

Commit

Permalink
Allow double-closing of FSTranslog
Browse files Browse the repository at this point in the history
the translog might be reused across engines which is currently a problem
in the design such that we have to allow calls to `close` more than once.
This moves the closed check for snapshot on the actual file to exit the loop.

Relates to #10807
  • Loading branch information
s1monw committed Apr 26, 2015
1 parent f87fb95 commit 2c510f0
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 16 deletions.
Expand Up @@ -239,6 +239,11 @@ public Path getPath() {
return channelReference.file();
}

@Override
public boolean closed() {
return this.closed.get();
}

class WrapperOutputStream extends OutputStream {

@Override
Expand Down
28 changes: 13 additions & 15 deletions src/main/java/org/elasticsearch/index/translog/fs/FsTranslog.java
Expand Up @@ -94,8 +94,6 @@ public void onRefreshSettings(Settings settings) {

private final ApplySettings applySettings = new ApplySettings();

private final AtomicBoolean closed = new AtomicBoolean(false);

@Inject
public FsTranslog(ShardId shardId, @IndexSettings Settings indexSettings, IndexSettingsService indexSettingsService,
BigArrays bigArrays, ShardPath shardPath) throws IOException {
Expand Down Expand Up @@ -141,16 +139,14 @@ public void updateBuffer(ByteSizeValue bufferSize) {

@Override
public void close() throws IOException {
if (closed.compareAndSet(false, true)) {
if (indexSettingsService != null) {
indexSettingsService.removeListener(applySettings);
}
rwl.writeLock().lock();
try {
IOUtils.close(this.trans, this.current);
} finally {
rwl.writeLock().unlock();
}
if (indexSettingsService != null) {
indexSettingsService.removeListener(applySettings);
}
rwl.writeLock().lock();
try {
IOUtils.close(this.trans, this.current);
} finally {
rwl.writeLock().unlock();
}
}

Expand Down Expand Up @@ -358,13 +354,15 @@ public Location add(Operation operation) throws TranslogException {
@Override
public FsChannelSnapshot snapshot() throws TranslogException {
while (true) {
if (closed.get()) {
throw new TranslogException(shardId, "translog is already closed");
}
FsTranslogFile current = this.current;
FsChannelSnapshot snapshot = current.snapshot();
if (snapshot != null) {
return snapshot;
}
if (current.closed() && this.current == current) {
// check if we are closed and if we are still current - then this translog is closed and we can exit
throw new TranslogException(shardId, "current translog is already closed");
}
Thread.yield();
}
}
Expand Down
Expand Up @@ -82,4 +82,6 @@ public static Type fromString(String type) throws ElasticsearchIllegalArgumentEx
TranslogStream getStream();

public Path getPath();

public boolean closed();
}
Expand Up @@ -182,4 +182,10 @@ public void reuse(FsTranslogFile other) {
public void updateBufferSize(int bufferSize) throws TranslogException {
// nothing to do here...
}

@Override
public boolean closed() {
return this.closed.get();
}

}
Expand Up @@ -340,7 +340,7 @@ public void testSnapshotOnClosedTranslog() throws IOException {
Translog.Snapshot snapshot = translog.snapshot();
fail("translog is closed");
} catch (TranslogException ex) {
assertEquals(ex.getMessage(), "translog is already closed");
assertEquals(ex.getMessage(), "current translog is already closed");
}
}

Expand Down

0 comments on commit 2c510f0

Please sign in to comment.