spring-integration icon indicating copy to clipboard operation
spring-integration copied to clipboard

duplicate key value violates unique constraint "int_lock_pk"

Open dkwakkel opened this issue 3 years ago • 4 comments

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 avatar Jul 28 '22 08:07 dkwakkel

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 avatar Jul 28 '22 17:07 artembilan

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!

dkwakkel avatar Aug 01 '22 16:08 dkwakkel

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!

artembilan avatar Aug 01 '22 16:08 artembilan

Setters is fine with me.

dkwakkel avatar Aug 01 '22 18:08 dkwakkel