Skip to content

Deadlock found when trying to get lock #1083

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

Closed
TorosyanV opened this issue May 30, 2018 · 9 comments
Closed

Deadlock found when trying to get lock #1083

TorosyanV opened this issue May 30, 2018 · 9 comments
Assignees
Labels
status: duplicate A duplicate of another issue

Comments

@TorosyanV
Copy link

TorosyanV commented May 30, 2018

I can't reproduce issue
spring session version 1.3.2
spring session-jdbs version 1.3.2
spring security version 1.5.12
spring web-socket 4.3.16
every day we had same issue in production environment
PreparedStatementCallback; SQL [INSERT INTO SPRING_SESSION_ATTRIBUTES(SESSION_ID, ATTRIBUTE_NAME, ATTRIBUTE_BYTES) VALUES (?, ?, ?)]; Deadlock found when trying to get lock; try restarting transaction; nested exception is java.sql.BatchUpdateException: Deadlock found when trying to get lock; try restarting transaction

@TorosyanV
Copy link
Author

In class WebSocketRegistryListener

there is method

private void afterConnectionEstablished(WebSocketSession wsSession) {
		Principal principal = wsSession.getPrincipal();
		if (principal == null) {
			return;
		}

		String httpSessionId = getHttpSessionId(wsSession);
		registerWsSession(httpSessionId, wsSession);
	}

and in class SessionRepositoryFilter<S extends ExpiringSession>

@Override
	protected void doFilterInternal(HttpServletRequest request,
			HttpServletResponse response, FilterChain filterChain)
			throws ServletException, IOException {
		request.setAttribute(SESSION_REPOSITORY_ATTR, this.sessionRepository);

		SessionRepositoryRequestWrapper wrappedRequest = new SessionRepositoryRequestWrapper(
				request, response, this.servletContext);
		SessionRepositoryResponseWrapper wrappedResponse = new SessionRepositoryResponseWrapper(
				wrappedRequest, response);

		HttpServletRequest strategyRequest = this.httpSessionStrategy
				.wrapRequest(wrappedRequest, wrappedResponse);
		HttpServletResponse strategyResponse = this.httpSessionStrategy
				.wrapResponse(wrappedRequest, wrappedResponse);

		try {
			filterChain.doFilter(strategyRequest, strategyResponse);
		}
		finally {
			wrappedRequest.commitSession();
		}
	}

Maybe there is concurrent session insertion ?

@vpavic vpavic self-assigned this May 30, 2018
@vpavic vpavic added the for: stack-overflow A question that's better suited to stackoverflow.com label May 30, 2018
@vpavic
Copy link
Contributor

vpavic commented May 30, 2018

Hi @TorosyanV - I saw you also posted some comments on #1027.

Are you using MySQL in a multi-master setup (like Galera), and did you take a look at other related issues (#676, #838) that were linked in this comment?

@vpavic vpavic added the status: waiting-for-feedback We need additional information before we can continue label May 30, 2018
@TorosyanV
Copy link
Author

Hi @vpavic , I'm using MySQL single instance. Currently I'm trying to reproduce this issue. Maybe it's related with spring security. Actually I don't know.

@TorosyanV
Copy link
Author

Deadlock happen when two transactions wait on each other to acquire a lock. Example:

Tx 1: lock A, then B
Tx 2: lock B, then A

There are numerous questions and answers about deadlocks. Each time you insert/update/or delete a row, a lock is acquired. To avoid deadlock, you must then make sure that concurrent transactions don't update row in an order that could result in a deadlock. Generally speaking, try to acquire lock always in the same order even in different transaction (e.g. always table A first, then table B).

Another reason for deadlock in database can be missing indexes. When a row is inserted/update/delete, the database needs to check the relational constraints, that is, make sure the relations are consistent. To do so, the database needs to check the foreign keys in the related tables. It might result in other lock being acquired than the row that is modified. Be sure then to always have index on the foreign keys (and of course primary keys), otherwise it could result in a table lock instead of a row lock. If table lock happen, the lock contention is higher and the likelihood of deadlock increases.

@TorosyanV
Copy link
Author

TorosyanV commented May 30, 2018

I think we cane change delete DELETE_SESSIONS_BY_LAST_ACCESS_TIME_QUERY to like this

private static final String DELETE_SESSIONS_BY_LAST_ACCESS_TIME_QUERY =
			"DELETE FROM %TABLE_NAME% " +
					"WHERE MAX_INACTIVE_INTERVAL < (? - LAST_ACCESS_TIME) / 1000";

private static final String DELETE_SESSIONS_BY_LAST_ACCESS_TIME_QUERY =
			"DELETE FROM %TABLE_NAME% " +"WHERE SESSION_ID in ( SELECT SESSION_ID from  %TABLE_NAME% where  MAX_INACTIVE_INTERVAL < (? - LAST_ACCESS_TIME) / 1000)";

I think it can solve

@TorosyanV
Copy link
Author

TorosyanV commented May 30, 2018

@vpavic
I added HttpSessionConfig and CustomJdbcOperationsSessionRepository with some log and deployed to production. Next time will log cleanUpExpiredSessions.
In this scenario we can understand it is result of deleting expired sessions or not.

@EnableJdbcHttpSession
public class HttpSessionConfig {

  @Bean
  public CustomJdbcOperationsSessionRepository sessionRepository(
      @Qualifier("springSessionJdbcOperations") JdbcOperations jdbcOperations,
      PlatformTransactionManager transactionManager) {
    return new CustomJdbcOperationsSessionRepository(
        jdbcOperations, transactionManager);
  }

}

public class CustomJdbcOperationsSessionRepository
    extends JdbcOperationsSessionRepository {


  private static final Logger logger = LoggerFactory
      .getLogger(CustomJdbcOperationsSessionRepository.class);

  public CustomJdbcOperationsSessionRepository(JdbcOperations jdbcOperations,
      PlatformTransactionManager transactionManager) {
    super(jdbcOperations, transactionManager);
  }

  @Override
  @Scheduled(fixedRate = 240 * 60 * 1000)
  public void cleanUpExpiredSessions() {
    logger.info("cleanUpExpiredSessions");
    super.cleanUpExpiredSessions();
  }

}

@vpavic
Copy link
Contributor

vpavic commented May 30, 2018

Thanks for the feedback and looking into this @TorosyanV.

OK, you're not using MySQL in multi-master setup so that makes your scenario a bit different from the others. How many instances of your app do you run in a cluster?

Before changing SQL query used for deletion of expired sessions, we need to carefully analyse impact on all major relational databases. Thing is, deadlock problems have almost exclusively been reported by MySQL users.

@vpavic
Copy link
Contributor

vpavic commented Nov 12, 2018

Duplicate of #1027

@vpavic vpavic marked this as a duplicate of #1027 Nov 12, 2018
@vpavic vpavic closed this as completed Nov 12, 2018
@vpavic vpavic added status: duplicate A duplicate of another issue and removed status: waiting-for-feedback We need additional information before we can continue labels Nov 12, 2018
@vpavic vpavic removed the for: stack-overflow A question that's better suited to stackoverflow.com label Nov 14, 2020
@vpavic
Copy link
Contributor

vpavic commented Nov 14, 2020

Marking this as a duplicate of #838. Please subscribe to that issue to track further updates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: duplicate A duplicate of another issue
Projects
None yet
Development

No branches or pull requests

2 participants