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

Moved transaction handling into Handle instead of @Transactional #4966

Merged
merged 4 commits into from
Aug 2, 2024
Merged
Show file tree
Hide file tree
Changes from 3 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 @@ -2,13 +2,11 @@

import io.agroal.api.AgroalDataSource;
import io.apicurio.registry.storage.error.RegistryStorageException;
import io.apicurio.registry.storage.impl.sql.jdb.Handle;
import io.apicurio.registry.storage.impl.sql.jdb.HandleAction;
import io.apicurio.registry.storage.impl.sql.jdb.HandleCallback;
import io.apicurio.registry.storage.impl.sql.jdb.HandleImpl;
import org.slf4j.Logger;

import java.io.IOException;
import java.sql.SQLException;
import java.util.HashMap;
import java.util.Map;
Expand All @@ -32,35 +30,56 @@

@Override
public <R, X extends Exception> R withHandle(HandleCallback<R, X> callback) throws X {
/*
* Handles are cached and reused if calls to this method are nested. Make sure that all nested uses of
* a handle are either within a transaction context, or without one. Starting a transaction with a
* nested handle will cause an exception.
*/
LocalState state = state();
try {
if (get().handle == null) {
get().handle = new HandleImpl(dataSource.getConnection());
// Create a new handle, or throw if one already exists (only one handle allowed at a time)
if (state.handle == null) {
state.handle = new HandleImpl(dataSource.getConnection());
} else {
get().level++;
throw new RegistryStorageException("Attempt to acquire a nested DB Handle.");

Check warning on line 39 in app/src/main/java/io/apicurio/registry/storage/impl/sql/AbstractHandleFactory.java

View workflow job for this annotation

GitHub Actions / Qodana Community for JVM

'throw' caught by containing 'try' statement

`throw` caught by containing 'try' statement
}
return callback.withHandle(get().handle);

// Invoke the callback with the handle. This will either return a value (success)
// or throw some sort of exception.
return callback.withHandle(state.handle);
} catch (SQLException e) {
// If a SQL exception is thrown, set the handle to rollback.
state.handle.setRollback(true);
// Wrap the SQL exception.
throw new RegistryStorageException(e);
} catch (Exception e) {

Check warning on line 50 in app/src/main/java/io/apicurio/registry/storage/impl/sql/AbstractHandleFactory.java

View workflow job for this annotation

GitHub Actions / Qodana Community for JVM

Overly broad 'catch' block

'catch' of `Exception` is too broad, masking exceptions 'RegistryStorageException' and 'X'
// If any other exception is thrown, also set the handle to rollback.
if (state.handle != null) {
state.handle.setRollback(true);
}
throw e;

Check warning on line 55 in app/src/main/java/io/apicurio/registry/storage/impl/sql/AbstractHandleFactory.java

View workflow job for this annotation

GitHub Actions / Qodana Community for JVM

Prohibited exception thrown

Prohibited exception 'Exception' thrown
} finally {
if (get().level > 0) {
get().level--;
} else {
try {
LocalState partialState = get();
if (partialState.handle != null) {
partialState.handle.close();
// Commit or rollback the transaction
try {
if (state.handle != null) {
if (state.handle.isRollback()) {
log.trace("Rollback: {} #{}", state.handle.getConnection(),
state.handle.getConnection().hashCode());
state.handle.getConnection().rollback();
} else {
log.trace("Commit: {} #{}", state.handle.getConnection(),
state.handle.getConnection().hashCode());
state().handle.getConnection().commit();
}
} catch (IOException ex) {
// Nothing we can do
log.error("Could not close a database handle", ex);
} finally {
local.get().remove(dataSourceId);
}
} catch (Exception e) {

Check warning on line 70 in app/src/main/java/io/apicurio/registry/storage/impl/sql/AbstractHandleFactory.java

View workflow job for this annotation

GitHub Actions / Qodana Community for JVM

Overly broad 'catch' block

'catch' of `Exception` is too broad, masking exception 'RuntimeException'
log.error("Could not release database connection/transaction", e);
}

// Close the connection
try {
if (state.handle != null) {
state.handle.close();
state.handle = null;

Check warning on line 78 in app/src/main/java/io/apicurio/registry/storage/impl/sql/AbstractHandleFactory.java

View workflow job for this annotation

GitHub Actions / Qodana Community for JVM

'null' assignment

'null' assigned to variable `state.handle`
}
} catch (Exception ex) {

Check warning on line 80 in app/src/main/java/io/apicurio/registry/storage/impl/sql/AbstractHandleFactory.java

View workflow job for this annotation

GitHub Actions / Qodana Community for JVM

Overly broad 'catch' block

'catch' of `Exception` is too broad, masking exception 'IOException'
// Nothing we can do
log.error("Could not close a database connection.", ex);
}
}
}
Expand All @@ -84,14 +103,11 @@
});
}

private LocalState get() {
private LocalState state() {
return local.get().computeIfAbsent(dataSourceId, k -> new LocalState());
}

private static class LocalState {

Handle handle;

int level;
HandleImpl handle;
}
}
Loading
Loading