RakambdaOrg/ChannelPointsMiner

Error saving user prediction

Rakambda opened this issue · 3 comments

Description

I don't have the context of how it happened, but my guess is that this user send messages in the chat quite fast or at the same time as an even update event.
We should handle concurrency properly.

Related changes from #259

Version / commit

cebea25

Relevant log output

EventHandler threw an exception
java.sql.SQLIntegrityConstraintViolationException: (conn=41738) Duplicate entry 'xxxxx' for key 'Username'
at org.mariadb.jdbc.export.ExceptionFactory.createException(ExceptionFactory.java:288)
at org.mariadb.jdbc.export.ExceptionFactory.create(ExceptionFactory.java:368)
at org.mariadb.jdbc.message.ClientMessage.readPacket(ClientMessage.java:137)
at org.mariadb.jdbc.client.impl.StandardClient.readPacket(StandardClient.java:833)
at org.mariadb.jdbc.client.impl.StandardClient.readResults(StandardClient.java:772)
at org.mariadb.jdbc.client.impl.StandardClient.readResponse(StandardClient.java:691)
at org.mariadb.jdbc.client.impl.StandardClient.execute(StandardClient.java:634)
at org.mariadb.jdbc.ClientPreparedStatement.executeInternal(ClientPrep aredStatement.java:95)
at org.mariadb.jdbc.ClientPreparedStatement.executeLargeUpdate(ClientPreparedStatement.java:334)
at org.mariadb.jdbc.ClientPreparedStatement.executeUpdate(ClientPreparedStatement.java:311)
at com.zaxxer.hikari.pool.ProxyPreparedStatement.executeUpdate(ProxyPreparedStatement.java:61)
at com.zaxxer.hikari.pool.HikariProxyPreparedStatement.executeUpdate(HikariProxyPreparedStatement.java)
at fr.raksrinana.channelpointsminer.miner.database.BaseDatabase.addUserPrediction(BaseDatabase.java:104)
at fr.raksrinana.channelpointsminer.miner.database.DatabaseEventHandler.onEventUpdatedEvent(DatabaseEventHandler.java:49)
at fr.raksrinana.channelpointsminer.miner.event.EventHandlerAdapter.onEvent(EventHandlerAdapter.java:82)
at fr.raksrinana.channelpointsminer.miner.miner.Miner.lambda$onEvent$1(Miner.java:325)
at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:539)
at java.base/java.util.concurrent.FutureTa sk.run(FutureTask.java:264)
at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
at java.base/java.lang.Thread.run(Thread.java:833)

Should be fixed with 1f7d1cc

I think changing
INSERT INTO PredictionUser(Username) VALUES (?) RETURNING ID
to
INSERT INTO PredictionUser(Username) VALUES (?) ON DUPLICATE KEY UPDATE ID=ID RETURNING ID
should do the trick as well.

It could work but but don't really like the idea.

In the code we first check if it exists then create it if it doesn't. So when we create it, semantically it has to not exist. Though the INSERT INTO ON KEY DUPLICATE could be used to replace both SELECT and INSERT.

However, I think that when you do this and you have a duplicate key, it still increments the AUTO_INCREMENT counter in the back (wouldn't cause issues but would be better if it doesn't do this).

Another approach could be to do the lock directly in DB with a transaction.