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

duplicate key value violates unique constraint "int_lock_pk" #3866

Closed
dkwakkel opened this issue Jul 28, 2022 · 6 comments · Fixed by #8606
Closed

duplicate key value violates unique constraint "int_lock_pk" #3866

dkwakkel opened this issue Jul 28, 2022 · 6 comments · Fixed by #8606

Comments

@dkwakkel
Copy link

In what version(s) of Spring Integration are you seeing this issue?

For example:

5.5.14

Describe the bug

When using DefaultLockRepository The following error is shown in postgres log under high load:

STATEMENT: INSERT INTO INT_LOCK (REGION, LOCK_KEY, CLIENT_ID, CREATED_DATE) VALUES ($1, $2, $3, $4)
ERROR: duplicate key value violates unique constraint "int_lock_pk"

To Reproduce

Use DefaultLockRepository with high load with postgres

Expected behavior

No error

** Workaround **
We use below workaround, but would be nice if this can be solved somewhere in spring integration itself:

		DefaultLockRepository repository = new DefaultLockRepository(datasource) {

			@Override
			public void afterPropertiesSet() {
				// Postgres specific fix to have no 'duplicate key value violates unique constraint "int_lock_pk"' in the log.
				if(asList(environment.getActiveProfiles()).contains("postgres")) {
					addOnConflictDoNothing();
				}
				super.afterPropertiesSet();
			}

			private void addOnConflictDoNothing() {
				String fieldName = "insertQuery";
				try {
					Field field = DefaultLockRepository.class.getDeclaredField(fieldName);
					field.setAccessible(true);
					field.set(this, field.get(this) + " ON CONFLICT DO NOTHING");
				} catch(Exception e) {
					log.warn("Unable to change spring integration '" + fieldName + "' query", e);
				}
			}
		};
		return repository;
	}
@dkwakkel dkwakkel added status: waiting-for-triage The issue need to be evaluated and its future decided type: bug labels Jul 28, 2022
@artembilan
Copy link
Member

May we see the whole stack trace, please?
Doesn't look like that duplicate key value violates unique constraint is translated to the DataIntegrityViolationException some way...
This is not the first time I see that PostgreSQL drive throws non-standard error which can be translated properly to respective exception in the JdbcTemplate.

I think we can fix that DefaultLockRepository exposing all the statements as setters, so you will be able to do something like:

repository.setInsertQuery("INSERT INTO %sLOCK (REGION, LOCK_KEY, CLIENT_ID, CREATED_DATE) VALUES (?, ?, ?, ?) ON CONFLICT DO NOTHING");

If that is really what is recommended for handling duplicate keys in PostgreSQL ...

Another way would be to let this DefaultLockRepository to be extendable, so you could override that acquire(String lock).

In 6.0, though, we added this:

	/**
	 * Set a {@link PlatformTransactionManager} for operations.
	 * Otherwise, a primary {@link PlatformTransactionManager} bean is obtained
	 * from the application context.
	 * @param transactionManager the {@link PlatformTransactionManager} to use.
	 * @since 6.0
	 */
	public void setTransactionManager(PlatformTransactionManager transactionManager) {

where a transactionDefinition.setIsolationLevel(TransactionDefinition.ISOLATION_SERIALIZABLE); i used for that INSERT operation.

So, WDYT? What would be the best way to address this concern from the framework perspective?
Thanks

@artembilan artembilan added status: waiting-for-reporter Needs a feedback from the reporter in: jdbc and removed status: waiting-for-triage The issue need to be evaluated and its future decided labels Jul 28, 2022
@dkwakkel
Copy link
Author

dkwakkel commented Aug 1, 2022

The error is logged only in postgres itself, there is no stacktrace in our application.

I think the workaround is fine because the result is 0 rows are updated, by which acquire will retry (please correct me if I am wrong).

It would be great if it works out of the box in the best possible way. Is it an idea to ask from different db vendors input on what the best queries are, and then have these as defaults? But that after years being in the field I am the first asking for such a thing already indicates that this is probably not needed (it already works out of the box fine). So think current workaround is good enough. When more people come with similar problems probably a better decision can be made what kind of change must be made.

From my side you can close this ticket. Thanks for the input!

@artembilan
Copy link
Member

ask from different db vendors input on what the best queries are

I doubt that any vendor provide such an info. And I guess that's a reason why even Hibernate has that Dialect abstraction.
When, by the way, even their PostgreSQLDialect does not mention any ON CONFLICT DO NOTHING.

We have some ChannelMessageStoreQueryProvider abstraction for the JdbcChannelMessageStorem but I find that as overhead and I feel like a plain setInsertQuery(), setDelete() etc. would be fully enough for extendability on the functionality like you are facing with PostgreSQL.

Although I still be upset that its driver does not throw any exceptions to the client 😢 ...

If you are OK with the fix for this issue as setters for those queries, then we won't close this as Works as Designed, but rather as Enhancement, so no one would ask for the same problem again, but just will be able to set their own query with any required vendor-specific hints!

@dkwakkel
Copy link
Author

dkwakkel commented Aug 1, 2022

Setters is fine with me.

@artembilan artembilan added type: enhancement and removed status: waiting-for-reporter Needs a feedback from the reporter type: bug labels Aug 1, 2022
@artembilan artembilan added this to the 6.0.x milestone Aug 1, 2022
@artembilan artembilan added the ideal-for-user-contribution An issue that would ideal for a user to get started with contributing. label Sep 8, 2022
@artembilan artembilan modified the milestones: 6.0.x, 6.1.x Jan 17, 2023
@umeshgiri8
Copy link

Hi @artembilan can I contribute to this?

@artembilan
Copy link
Member

Sure! You are welcome!

@artembilan artembilan self-assigned this Apr 28, 2023
@artembilan artembilan modified the milestones: 6.1.x, 6.1.0 Apr 28, 2023
@artembilan artembilan removed the ideal-for-user-contribution An issue that would ideal for a user to get started with contributing. label Apr 28, 2023
artembilan added a commit to artembilan/spring-integration that referenced this issue Apr 28, 2023
Fixes spring-projects#3866

Some RDBMS vendors (or their JDBC drivers) may just log the problem
without throwing an exception.

* Expose setters for UPDATE and INSERTS queries in the `DefaultLockRepository`
to let end-user to modify them respectively, e.g. be able to add a PostgreSQL
`ON CONFLICT DO NOTHING` hint.
* Refactor `DefaultLockRepository` to `Instant` instead of `LocalDateTime`
with zone offset.
* Refactor `ttl` property to `Duration` type
* Fix `dead-lock` typo to `deadlock`
garyrussell pushed a commit that referenced this issue May 1, 2023
Fixes #3866

Some RDBMS vendors (or their JDBC drivers) may just log the problem
without throwing an exception.

* Expose setters for UPDATE and INSERTS queries in the `DefaultLockRepository`
to let end-user to modify them respectively, e.g. be able to add a PostgreSQL
`ON CONFLICT DO NOTHING` hint.
* Refactor `DefaultLockRepository` to `Instant` instead of `LocalDateTime`
with zone offset.
* Refactor `ttl` property to `Duration` type
* Fix `dead-lock` typo to `deadlock`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants