Skip to content
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

Always close BigTable clients #8549

Merged
merged 2 commits into from
May 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,11 @@ public abstract class AbstractBigTableBackendTestFactory implements BackendTestF

@Override
public Backend createNewBackend() {
return createNewBackend(bigtableConfigBuilder().build(), true);
return createNewBackend(bigtableConfigBuilder().build());
}

@SuppressWarnings("ClassEscapesDefinedScope")
public BigTableBackend createNewBackend(
BigTableBackendConfig bigtableConfig, boolean closeClient) {
return new BigTableBackend(bigtableConfig, closeClient);
public Backend createNewBackend(BigTableBackendConfig bigtableConfig) {
return new BigTableBackend(bigtableConfig);
}

public Builder bigtableConfigBuilder() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,10 @@ public void productionLike() throws Exception {
BackendFactory<BigTableBackendConfig> factory =
PersistLoader.findFactoryByName(BigTableBackendFactory.NAME);
soft.assertThat(factory).isNotNull().isInstanceOf(BigTableBackendFactory.class);
RepositoryDescription repoDesc;

try (BigtableDataClient dataClient = testFactory.buildNewDataClient();
BigtableTableAdminClient tableAdminClient = testFactory.buildNewTableAdminClient()) {
RepositoryDescription repoDesc;
try (Backend backend =
factory.buildBackend(
BigTableBackendConfig.builder()
Expand All @@ -68,7 +68,10 @@ public void productionLike() throws Exception {
repoDesc = repositoryLogic.fetchRepositoryDescription();
soft.assertThat(repoDesc).isNotNull();
}
}

try (BigtableDataClient dataClient = testFactory.buildNewDataClient();
BigtableTableAdminClient tableAdminClient = testFactory.buildNewTableAdminClient()) {
try (Backend backend =
factory.buildBackend(
BigTableBackendConfig.builder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,7 @@ public class NoAdminClientBackendRepositoryTests extends AbstractBackendReposito
@BeforeEach
void noAdminClient() {
BigTableBackend b = (BigTableBackend) backend;
backend =
new BigTableBackend(
BigTableBackendConfig.builder().dataClient(b.client()).build(), false);
backend = new BigTableBackend(BigTableBackendConfig.builder().dataClient(b.client()).build());
}
}

Expand All @@ -67,13 +65,19 @@ public class TablePrefixes {
void tablePrefix() {
AbstractBigTableBackendTestFactory btFactory = (AbstractBigTableBackendTestFactory) factory;
try (BigTableBackend backendA =
btFactory.createNewBackend(
btFactory.bigtableConfigBuilder().tablePrefix(Optional.of("instanceA")).build(),
true);
(BigTableBackend)
btFactory.createNewBackend(
btFactory
.bigtableConfigBuilder()
.tablePrefix(Optional.of("instanceA"))
.build());
BigTableBackend backendB =
btFactory.createNewBackend(
btFactory.bigtableConfigBuilder().tablePrefix(Optional.of("instanceB")).build(),
true)) {
(BigTableBackend)
btFactory.createNewBackend(
btFactory
.bigtableConfigBuilder()
.tablePrefix(Optional.of("instanceB"))
.build())) {

BigtableTableAdminClient adminClientA = requireNonNull(backendA.adminClient());
BigtableTableAdminClient adminClientB = requireNonNull(backendB.adminClient());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,13 @@ public final class BigTableBackend implements Backend {
private final BigTableBackendConfig config;
private final BigtableDataClient dataClient;
private final BigtableTableAdminClient tableAdminClient;
private final boolean closeClient;

final String tableRefs;
final String tableObjs;
final TableId tableRefsId;
final TableId tableObjsId;

public BigTableBackend(@Nonnull BigTableBackendConfig config, boolean closeClient) {
public BigTableBackend(@Nonnull BigTableBackendConfig config) {
this.config = config;
this.dataClient = config.dataClient();
this.tableAdminClient = config.tableAdminClient();
Expand All @@ -76,7 +75,6 @@ public BigTableBackend(@Nonnull BigTableBackendConfig config, boolean closeClien
config.tablePrefix().map(prefix -> prefix + '_' + TABLE_OBJS).orElse(TABLE_OBJS);
this.tableRefsId = TableId.of(tableRefs);
this.tableObjsId = TableId.of(tableObjs);
this.closeClient = closeClient;
}

@Nonnull
Expand All @@ -102,28 +100,26 @@ public PersistFactory createFactory() {

@Override
public void close() {
if (closeClient) {
RuntimeException ex = null;
try {
dataClient.close();
} catch (Exception e) {
ex = new RuntimeException(e);
}
try {
if (tableAdminClient != null) {
tableAdminClient.close();
}
} catch (Exception e) {
if (ex == null) {
ex = new RuntimeException(e);
} else {
ex.addSuppressed(e);
}
RuntimeException ex = null;
try {
dataClient.close();
} catch (Exception e) {
ex = new RuntimeException(e);
}
try {
if (tableAdminClient != null) {
tableAdminClient.close();
}
if (ex != null) {
throw ex;
} catch (Exception e) {
if (ex == null) {
ex = new RuntimeException(e);
} else {
ex.addSuppressed(e);
}
}
if (ex != null) {
throw ex;
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,6 @@ public BigTableBackendConfig newConfigInstance() {
@Override
@Nonnull
public Backend buildBackend(@Nonnull BigTableBackendConfig config) {
return new BigTableBackend(config, false);
return new BigTableBackend(config);
}
}