-
Notifications
You must be signed in to change notification settings - Fork 0
Added TRANSACTION_ISOLATION_LEVEL in AbstractDBSpecificConnectorConfig #1
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
base: develop
Are you sure you want to change the base?
Conversation
| if (transactionIsolationLevel == null) { | ||
| transactionIsolationLevel = TransactionIsolationLevel.Level.TRANSACTION_SERIALIZABLE.name(); | ||
| } | ||
| return TransactionIsolationLevel.Level.valueOf(transactionIsolationLevel).name(); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we set the transactionIsolationLevel to TRANSACTION_READ_COMMITTED for normal roles just to ensure that the role is mapped to the right serialization level, even with incorrect user input?
//if role is SYSDBA or SYSOP it will map to read_committed. else serialized
return (!getRole().equals(ROLE_NORMAL)) ? TransactionIsolationLevel.Level.TRANSACTION_READ_COMMITTED.name() :
TransactionIsolationLevel.Level.valueOf(transactionIsolationLevel).name();
|
@Krish-cloudsufi You'd also need to put the Transaction Isolation Level in the additional arguments for all affected DB plugins. See how it's done in Oracle Connector Configuration for reference. |
| @Nullable | ||
| protected Integer port; | ||
|
|
||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove blank line
|
|
||
| public String getTransactionIsolationLevel() { | ||
| if (transactionIsolationLevel == null) { | ||
| transactionIsolationLevel = TransactionIsolationLevel.Level.TRANSACTION_READ_COMMITTED.name(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in other plugins we dont have a default isolation level set by us as it can differ db wise, return null if it is null
|
|
||
| **Transaction Isolation Level** The transaction isolation level of the databse connection | ||
| - TRANSACTION_READ_COMMITTED: No dirty reads. Non-repeatable reads and phantom reads are possible. | ||
| - TRANSACTION_SERIALIZABLE (default): No dirty reads. Non-repeatable and phantom reads are prevented. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mysql-plugin/docs/MySQL-connector.md
Outdated
|
|
||
| **Transaction Isolation Level** The transaction isolation level of the databse connection | ||
| - TRANSACTION_READ_COMMITTED: No dirty reads. Non-repeatable reads and phantom reads are possible. | ||
| - TRANSACTION_SERIALIZABLE (default): No dirty reads. Non-repeatable and phantom reads are prevented. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://dev.mysql.com/doc/refman/8.4/en/innodb-transaction-isolation-levels.html#isolevel_repeatable-read this is also not default for mysql
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this we are making default on UI then its fine
| public static final String JDBC_PLUGIN_NAME = "jdbcPluginName"; | ||
| public static final String JDBC_PLUGIN_TYPE = "jdbc"; | ||
| public static final String TRANSACTION_ISOLATION_LEVEL = "transactionIsolationLevel"; | ||
| public static final String ROLE = "role"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused variable ROLE ?
| } | ||
|
|
||
| public String getTransactionIsolationLevel() { | ||
| LOG.debug("Hi Krish Inside AbstractDBSpecificSourceConfig getTransactionIsolationLevel "); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this logger one testing is complete.
| } | ||
|
|
||
| public Map<String, String> getAdditionalArguments() { | ||
| LOG.debug("inside get Additional argument of AbstractDBConnectorConfig"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove after testing
Added support for specifying the transaction isolation level in database plugins by introducing a new optional property "transactionIsolationLevel" in the AbstractDBSpecificConnectorConfig class — the parent class for MySQL, PostgreSQL, and Oracle connector configurations.