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

Cohort generation on RedShift - serializable isolation violation on table Error: 1023 #1939

Open
anthonysena opened this issue Sep 13, 2021 · 16 comments
Labels
bug known issue Identifies known issues with the platform that are under investigation.

Comments

@anthonysena
Copy link
Collaborator

Expected behavior

When generating > 1 cohort on a single data source, the cohort generation processes finish without error on RedShift.

Actual behavior

Generating > 1 cohort on RedShift using WebAPI v2.10.0 generates the following exception:

org.springframework.jdbc.UncategorizedSQLException: StatementCallback; uncategorized SQLException for SQL [CREATE TABLE #Codesets  (codeset_id int NOT NULL,
  concept_id bigint NOT NULL
)
DISTSTYLE ALL; INSERT INTO #Codesets (codeset_id, concept_id)
SELECT 4 as codeset_id, c.concept_id FROM (select distinct I.concept_id FROM
( 
  select concept_id from cdm_truven_ccae_v1676.CONCEPT where concept_id in (45774435,44816332,40170911,1583722,793143,44506754)
UNION  select c.concept_id
  from cdm_truven_ccae_v1676.CONCEPT c
  join cdm_truven_ccae_v1676.CONCEPT_ANCESTOR ca on c.concept_id = ca.descendant_concept_id
  and ca.ancestor_concept_id in (45774435,44816332,40170911,1583722,793143,44506754)
  and c.invalid_reason is null

) I
LEFT JOIN
(
  select concept_id from cdm_truven_ccae_v1676.CONCEPT where concept_id in (36410293,36410289,1718706,1718711,1718604,1718712,1718713,1718705,1718709,21174087,1718605,1718606,44191345,36055923,36055919)
UNION  select c.concept_... [truncated] ... sr.inclusion_rule_mask = POWER(cast(2 as bigint),RuleTotal.total_rules)-1
) FC
; TRUNCATE TABLE #best_events; DROP TABLE #best_events; TRUNCATE TABLE #inclusion_rules; DROP TABLE #inclusion_rules; TRUNCATE TABLE #strategy_ends; DROP TABLE #strategy_ends; TRUNCATE TABLE #cohort_rows; DROP TABLE #cohort_rows; TRUNCATE TABLE #final_cohort; DROP TABLE #final_cohort; TRUNCATE TABLE #inclusion_events; DROP TABLE #inclusion_events; TRUNCATE TABLE #qualified_events; DROP TABLE #qualified_events; TRUNCATE TABLE #included_events; DROP TABLE #included_events; TRUNCATE TABLE #Codesets; DROP TABLE #Codesets]; 
SQL state [XX000]; error code [0]; ERROR: 1023
  Detail: Serializable isolation violation on table - 5840179, transactions forming the cycle are: 77581433, 77582607 (pid:21190); nested exception is com.amazon.redshift.util.RedshiftException: ERROR: 1023
  Detail: Serializable isolation violation on table - 5840179, transactions forming the cycle are: 77581433, 77582607 (pid:21190)

Steps to reproduce behavior

Generate > 1 cohort on RedShift using WebAPI v2.10.0.

Additional Notes

As part of the v2.10.0 release, we updated the RedShift JDBC drivers: https://github.com/OHDSI/WebAPI/pull/1925/files. As a test, we can try to roll back to using a 1.2.x build to see if that allows us to work-around this problem while we do a deeper dive.

@anthonysena
Copy link
Collaborator Author

I've done some testing with the v1.2 RedShift driver and that seems to be a work-around for now while we see what is happening with the v2 driver. We'll need to discuss putting this into v2.10.1 potentially.

@konstjar
Copy link
Contributor

@anthonysena did you just rolled back to last version that was used or different one?

@anthonysena
Copy link
Collaborator Author

I ultimately went back to the last driver that we had used - the changes are here for now: https://github.com/OHDSI/WebAPI/tree/issue-1939-redshift-driver-downgrade

@konstjar
Copy link
Contributor

@anthonysena @ssuvorov-fls worked on solution to fix the problem and go back to driver v1.0 but with IAM support. The functionality is available in "issue-1939-redshift-driver-downgrade-sdk" branch. The Redshift support was moved into separate profile "webapi-redshift", that's why you should mention in initialization and install steps.

We do the following steps in our environment

mvn initialize -Dredshift.classpath=/var/local/drivers/redshift-v1.2 -Pwebapi-redshift
mvn install -e -Dmaven.test.skip=true  -Dredshift.classpath=/var/local/drivers/redshift-v1.2 -Pwebapi-postgresql,webapi-redshift

For JDBC Driver, we used latest availabkle officially from AWS:
https://s3.amazonaws.com/redshift-downloads/drivers/jdbc/1.2.55.1083/RedshiftJDBC42-1.2.55.1083.zip

Could you please check this branch in your environment?

@anthonysena
Copy link
Collaborator Author

anthonysena commented Sep 20, 2021

@konstjar @ssuvorov-fls thank you both for investigating this problem and proposing the solution in the issue-1939-redshift-driver-downgrade-sdk branch. I had a few questions upon my review:

  • The original need to update this driver was to support IAM authentication - do I have that right? If so, I was looking at the AWS docs and noted that For authentication using AWS Identity and Access Management (IAM) credentials or identity provider (IdP) credentials, use Amazon Redshift JDBC driver version 1.2.8.1005 or later. Just want to note that in this thread in case we have to revisit the driver change. The v1.2.55.1083 should work well.
  • In the issue-1939-redshift-driver-downgrade-sdk branch, the com.amazon.redshift dependency was removed from the pom.xml in favor of using the maven command to install the profile instead. It appears that the webapi-redshift profile is doing 2 things: specifying the redshift.classpath property and also specify the redshift dependencies that are required for IAM support. Could we instead restore the 'com.amazon.redshift` dependency so that the driver is automatically downloaded and if someone wants to use IAM, they could follow the maven initalize/install steps above?

@konstjar
Copy link
Contributor

@anthonysena

  • You got it correctly in the first bullet. Amazon Redshift JDBC driver version 1.2.8.1005 or later should work well for IAM support.
  • I think it should work this way and this will not break RedShift support for everyone in this case. But I would let @ssuvorov-fls to test this approach.

@ssuvorov-fls
Copy link
Contributor

ssuvorov-fls commented Sep 20, 2021

@anthonysena
The driver can't be automatically downloaded because it is not located in maven repository. Maven central has only versions started from 2.0.0.3
So it must be installed first.
If we will restore the com.amazon.redshift dependency and call mvn compile without setting the required profile then we'll get error

@anthonysena
Copy link
Collaborator Author

@ssuvorov-fls could we use one of the versions listed on https://mvnrepository.com/artifact/com.amazon.redshift/redshift-jdbc4-no-awssdk? The most recent is v1.2.43.1067 - not sure if that poses any problems but it is > v1.2.8.

@anthonysena
Copy link
Collaborator Author

@anthonysena
Copy link
Collaborator Author

@ssuvorov-fls - thanks for the update here. I tested using the 1.2.55.1083 version of the driver and it also exhibited the same problem as originally reported. I'd suggest that we move back to 1.2.10.1009 since this is > 1.2.8 and should allow for the IAM support required.

@konstjar let me know if there are any objections and I can file the PR based on b1cf1d2

@anthonysena
Copy link
Collaborator Author

Thanks @ssuvorov-fls - can you file a PR? I'll review & approve. Thanks!

ssuvorov-fls added a commit that referenced this issue Oct 6, 2021
anthonysena added a commit that referenced this issue Oct 7, 2021
* redshift driver was downgraded to 1.2.10.1009 (#1939)

Co-authored-by: Sergey Suvorov <[email protected]>
ssuvorov-fls added a commit that referenced this issue Oct 12, 2021
@anthonysena
Copy link
Collaborator Author

A quick update on this issue: @chrisknoll and I worked together to figure out the RedShift JDBC drivers <= 1.2.45.1069 work when generating 2 cohorts in parallel on the same data source. When we moved to v1.2.47.1071 (and even later versions), we observed the error mentioned earlier in this issue.

Here is a link to this driver on Maven Central: https://mvnrepository.com/artifact/com.amazon.redshift/redshift-jdbc42-no-awssdk

m0nhawk pushed a commit to uc-cdis/WebAPI that referenced this issue Nov 1, 2021
* redshift driver was downgraded to 1.2.10.1009 (OHDSI#1939)

Co-authored-by: Sergey Suvorov <[email protected]>
ssuvorov-fls pushed a commit that referenced this issue Nov 8, 2021
* redshift driver was downgraded to 1.2.10.1009 (#1939)

Co-authored-by: Sergey Suvorov <[email protected]>
(cherry picked from commit 5454d85)
ssuvorov-fls added a commit that referenced this issue Jan 18, 2022
* Implemented logic of reusable components and their tagging

* renamed migration script

* Reusables implementation - fixes and additions (Atlas #2623)

* Cohort exit date text is wrong in Atlas > Prediction > Population settings #2622 (#1944)

(cherry picked from commit af00e1c)

* added versioning to reusables(#1939)

* Reusables implementation (#2623) WIP

* moved some permissions to role "moderator"(#1939)

* fixed after review(#1939)

* added endpoint for copying reusable(#1939)

* created endpoints for getting entities with assigned tags(#1946)

* enabled security for new endpoints(#1946)

* revert server.port configuration

* created endpoints for getting entities with assigned tags(#1946) - fixes

Co-authored-by: Sergey Suvorov <[email protected]>
Co-authored-by: Anton Abushkevich <[email protected]>
@anthonysena anthonysena removed this from the v2.10.1 milestone Jan 25, 2022
m0nhawk pushed a commit to uc-cdis/WebAPI that referenced this issue Jan 26, 2022
* Implemented logic of reusable components and their tagging

* renamed migration script

* Reusables implementation - fixes and additions (Atlas #2623)

* Cohort exit date text is wrong in Atlas > Prediction > Population settings #2622 (OHDSI#1944)

(cherry picked from commit af00e1c)

* added versioning to reusables(OHDSI#1939)

* Reusables implementation (#2623) WIP

* moved some permissions to role "moderator"(OHDSI#1939)

* fixed after review(OHDSI#1939)

* added endpoint for copying reusable(OHDSI#1939)

* created endpoints for getting entities with assigned tags(OHDSI#1946)

* enabled security for new endpoints(OHDSI#1946)

* revert server.port configuration

* created endpoints for getting entities with assigned tags(OHDSI#1946) - fixes

Co-authored-by: Sergey Suvorov <[email protected]>
Co-authored-by: Anton Abushkevich <[email protected]>
@konstjar
Copy link
Contributor

I think this was resolved and could be closed?

@chrisknoll
Copy link
Collaborator

I'm not sure: last I recall when I tried this with Sena was that we went through about a dozen different versions of the jdbc driver, and we continue to experience the error. @anthonysena , am I recalling that correctly?

@anthonysena
Copy link
Collaborator Author

That's correct @chrisknoll - looking at the current pom.xml, we're still using v1.2.10.1009 which does not exhibit this problem. We could note this as a "known issue" and leave it open since we've yet to address the root cause.

@anthonysena anthonysena added the known issue Identifies known issues with the platform that are under investigation. label May 3, 2022
@chrisknoll chrisknoll moved this from Needs Review to Under Review in Atlas/WebAPI Issue Triage Apr 18, 2023
@chrisknoll chrisknoll moved this from Under Review to Review Complete in Atlas/WebAPI Issue Triage Apr 18, 2023
@konstjar
Copy link
Contributor

konstjar commented Aug 29, 2024

Long time passed since last discussion. We have few same issues on Redshift appeared again but in pure R code using the DatabaseConnector using v2.x driver version. The DatabaseConnector package has JDBC driver v2.x.

Just for visibility purposes and general discussion I would like to ask @schuemie here if there are any reporters of the same "serializable isolation violation" problem and if you can propose any solution for this?
From our research, it's possible to alter database in Redshift to enable snapshot isolation. I just wonder if it's the right way to go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug known issue Identifies known issues with the platform that are under investigation.
Projects
No open projects
Status: Review Complete
Development

No branches or pull requests

5 participants