You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2021/07/07 20:30:37 UTC
[GitHub] [druid] santiagoguinle opened a new issue #11418: "FOR UPDATE" sql command is in the wrong place
santiagoguinle opened a new issue #11418:
URL: https://github.com/apache/druid/issues/11418
"FOR UPDATE" is not compatible with SQL SERVER or other several DB, this should be erased from here and use another strategy for locking rows
https://github.com/apache/druid/blob/d5e8d4d68007899df17f390a756422d511d746d5/server/src/main/java/org/apache/druid/metadata/SQLMetadataConnector.java#L476
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] FrankChen021 commented on issue #11418: "FOR UPDATE" sql command is in the wrong place
Posted by GitBox <gi...@apache.org>.
FrankChen021 commented on issue #11418:
URL: https://github.com/apache/druid/issues/11418#issuecomment-878728105
A simplest but not the graceful way to fix it is to override `compareAndSwap` in SQLServer extension.
A better method is to add an abstract `selectForUpdate` function in `SQLMetadataConnector` to allow different database implementations to implement in their own ways.
I'm also thinking about if it's possible to use optimistic lock mechanism to address the problem globally.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] santiagoguinle commented on issue #11418: "FOR UPDATE" sql command is in the wrong place
Posted by GitBox <gi...@apache.org>.
santiagoguinle commented on issue #11418:
URL: https://github.com/apache/druid/issues/11418#issuecomment-876485427
@FrankChen021 thanks for your answer! i am not also a SQLServer expert but i found some explanations about this issue
Acording to this: https://stackoverflow.com/questions/1483725/select-for-update-with-sql-server
An aproximated solution for "FOR UPDATE" in SQLServer is using "WITH (UPDLOCK)" but in fact if you want to just lock some specific rows then you need to define an index and use it on the same SQL expression.
For example create an index like this:
CREATE INDEX TBLINDEX ON TBL ( id )
and then lock only row with id 10 doing this:
SELECT * FROM TBL WITH (UPDLOCK, INDEX(TBLINDEX)) WHERE id=10.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] FrankChen021 commented on issue #11418: "FOR UPDATE" sql command is in the wrong place
Posted by GitBox <gi...@apache.org>.
FrankChen021 commented on issue #11418:
URL: https://github.com/apache/druid/issues/11418#issuecomment-876878005
I checked the 'FOR UPDATE' code change, it was added by #7661. And since SQLServer is supported by a community contributed extension, there's no integration tests so this problem was not exposed when the PR was done.
If this problem blocks your work, I'm suggesting you to fix it by your own if SQLServer is the only one option in your case. Or you could use MySQL/Postgre which are fully tested with Druid.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] FrankChen021 commented on issue #11418: "FOR UPDATE" sql command is in the wrong place
Posted by GitBox <gi...@apache.org>.
FrankChen021 commented on issue #11418:
URL: https://github.com/apache/druid/issues/11418#issuecomment-876082068
I'm not familiar with SQLServer, according to its document, it seems that `with(rowlock)` in SQLServer is equivalent to `FOR UPDATE` in MySQL, right ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] santiagoguinle commented on issue #11418: "FOR UPDATE" sql command is in the wrong place
Posted by GitBox <gi...@apache.org>.
santiagoguinle commented on issue #11418:
URL: https://github.com/apache/druid/issues/11418#issuecomment-877252588
Well, at least for now i am following your recommendation.
i already made the test: i downloaded the code, changed it (just deleted the "FOR UPDATE" string), recompiled, configured to work with SQLServer and works perfect. Aparently is the only line of code that brings incompatibility. i even tryied some ingestion tasks and everything went great. Of course, probably it would be necesary to make a bigger change in order to maintain the functionality of locking rows.
I also notice that Druid indeed has several files in his core dedicated to deal with SQLServer.
But this "bug" was writed in a core class file, so an extension doesn't look like the place to fix it.
So, i am pretty sure that #7661 was a great improvement and fix some problems but also creates a incompatibility with a great DB Engine.
I dont do JAVA programming, so meaby im not the correct person for helping with this...
BUT i will be happy to help in order to keep improving Druid, so let me know if i can do something.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org