[PLUGIN-1779] Add TRANSACTION_ISOLATION_LEVEL config in MySQL, PostgreSQL & SQL Server plugins #583
Conversation
sgarg-CS
changed the title
|
Thank you so much for this. 🥇 We've been facing issues with this, and I was looking for a solution. I am happy that it has already been implemented. Looking forward to it! 🤞 |
database-commons/src/main/java/io/cdap/plugin/db/connector/AbstractDBSpecificConnector.java
Outdated
Show resolved
Hide resolved
database-commons/src/main/java/io/cdap/plugin/db/connector/AbstractDBConnectorConfig.java
Outdated
Show resolved
Hide resolved
| @@ -49,6 +51,7 @@ public abstract class AbstractDBSpecificSourceConfig extends PluginConfig implem | |||
| public static final String DATABASE = "database"; | |||
| public static final String FETCH_SIZE = "fetchSize"; | |||
| public static final String DEFAULT_FETCH_SIZE = "1000"; | |||
| public static final Logger LOG = LoggerFactory.getLogger(AbstractDBSpecificSourceConfig.class); | |||
There was a problem hiding this comment.
This is not being used anywhere why do we need to add this?
...ase-commons/src/main/java/io/cdap/plugin/db/connector/AbstractDBSpecificConnectorConfig.java
Outdated
Show resolved
Hide resolved
mysql-plugin/src/main/java/io/cdap/plugin/mysql/MysqlSource.java
Outdated
Show resolved
Hide resolved
postgresql-plugin/src/main/java/io/cdap/plugin/postgres/PostgresSource.java
Outdated
Show resolved
Hide resolved
postgresql-plugin/src/main/java/io/cdap/plugin/postgres/PostgresSource.java
Outdated
Show resolved
Hide resolved
postgresql-plugin/src/main/java/io/cdap/plugin/postgres/PostgresConnectorConfig.java
Outdated
Show resolved
Hide resolved
Please add testing evidence for mySql and postgres. |
| @Description("The transaction isolation level for the database session.") | ||
| @Macro | ||
| @Nullable | ||
| protected String transactionIsolationLevel; |
There was a problem hiding this comment.
isn't this class also used by SQLServer?
There was a problem hiding this comment.
Yes, it is. Should we hide the property from SQLServer Plugins for now? OR
add this property to the respective connector configuration classes of PostgreSQL and MySQL plugins and remove from parent connector config?
There was a problem hiding this comment.
why not implement it in SQLServer as well?
psainics
force-pushed
the
patch/PLUGIN-1779
branch
from
aa2f77c to
d30d86c
Compare
|
|
|
sgarg-CS
changed the title
| "TRANSACTION_REPEATABLE_READ", | ||
| "TRANSACTION_SERIALIZABLE" | ||
| ], | ||
| "default": "TRANSACTION_SERIALIZABLE" |
There was a problem hiding this comment.
Change this to read commited as the default value as mentioned in the documentation.
There was a problem hiding this comment.
@ritwiksahani @itsankit-google Should we keep the default type as Serializable, which is currently used for all plugins, or should we change it to match the default isolation level supported by the database?
There was a problem hiding this comment.
Is Serializable used for all plugins? I don't think so, correct me if I am wrong but we don't set transaction isolation level for other plugins currently. In that case it would be using the default isolation level of the DB.
We should maintain backward compatibility in terms of behaviour and have the defauilt isolation level match the DB one.
There was a problem hiding this comment.
By default, we were setting TRANSACTION_SERIALIZABLE for all plugins except Oracle, ref:
There was a problem hiding this comment.
Got it, agreed then in that case we should keep SERIALIZABLE as default and not change the behaviour.
There was a problem hiding this comment.
Changed the default value to TRANSACTION_SERIALIZABLE 0881eb7
Added support for SQL Server as well. |
Testing evidence MySQL PostgreSQL Oracle MS SQL Server |
…lugins Add transaction isolation level to PostgreSQL, MYSQL and MSSQL Plugins.
sgarg-CS
force-pushed
the
patch/PLUGIN-1779
branch
from
0881eb7 to
f4650e1
Compare
Add TRANSACTION_ISOLATION_LEVEL config in MySQL, PostgreSQL & SQL Server plugins
PLUGIN-1779
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, SQL Server and Oracle connector configurations.