You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dolphinscheduler.apache.org by GitBox <gi...@apache.org> on 2022/06/25 16:16:57 UTC

[GitHub] [dolphinscheduler] ruanwenjun opened a new issue, #10617: [Improvement][Master] Remove the extra check of slot

ruanwenjun opened a new issue, #10617:
URL: https://github.com/apache/dolphinscheduler/issues/10617

   ### Search before asking
   
   - [X] I had searched in the [issues](https://github.com/apache/dolphinscheduler/issues?q=is%3Aissue) and found no similar feature requirement.
   
   
   ### Description
   
   We have a many places to check the slot of master.
   
   This is not needed, since this will not gurantee the command to be executed by only one master.
   We already have the database transaction to deal with this case. When we transform the command to processInstance, we will delete the command at one transaction, if the delete failed, the generate of processInstance will also failed.
   
   
   ### Use case
   
   _No response_
   
   ### Related issues
   
   _No response_
   
   ### Are you willing to submit a PR?
   
   - [X] Yes I am willing to submit a PR!
   
   ### Code of Conduct
   
   - [X] I agree to follow this project's [Code of Conduct](https://www.apache.org/foundation/policies/conduct)
   


-- 
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@dolphinscheduler.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [dolphinscheduler] caishunfeng commented on issue #10617: [Improvement][Master] Remove the extra check of slot

Posted by GitBox <gi...@apache.org>.
caishunfeng commented on issue #10617:
URL: https://github.com/apache/dolphinscheduler/issues/10617#issuecomment-1166768328

   Hi @ruanwenjun I have a different point, although  we already have the database transaction to gurantee the command just be consumed once, but it's not a lightweight operations and will increase the database pressure, so I think the slot check, as the pre-check,  is still necessary. WDYT? 


-- 
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@dolphinscheduler.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [dolphinscheduler] caishunfeng commented on issue #10617: [Improvement][Master] Remove the extra check of slot

Posted by GitBox <gi...@apache.org>.
caishunfeng commented on issue #10617:
URL: https://github.com/apache/dolphinscheduler/issues/10617#issuecomment-1169492236

   > > I have a different point, although we already have the database transaction to gurantee the command just be consumed once, but it's not a lightweight operations and will increase the database pressure, so I think the slot check, as the pre-check, is still necessary. WDYT
   > 
   > We will check the slot when query from the database, so in most of the time, there will not database rollback. My concern is that is we use the `slotCheck` here, some user may think this method can guarantee the command be consumed safe by one master. So if we really don't want to remove these, we still need to add a comment to tell this method is not safe.
   
   Yes, agree with you, we should add some comments for it.


-- 
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@dolphinscheduler.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [dolphinscheduler] ruanwenjun commented on issue #10617: [Improvement][Master] Remove the extra check of slot

Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on issue #10617:
URL: https://github.com/apache/dolphinscheduler/issues/10617#issuecomment-1166775235

   > I have a different point, although we already have the database transaction to gurantee the command just be consumed once, but it's not a lightweight operations and will increase the database pressure, so I think the slot check, as the pre-check, is still necessary. WDYT
   
   We will check the slot when query from the database, so in most of the time, there will not database rollback. My concern is that is we use the `slotCheck` here, some user may think this method can guarantee the command be consumed safe by one master. So if we really don't want to remove these, we still need to add a comment to tell this method is not safe.


-- 
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@dolphinscheduler.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [dolphinscheduler] ruanwenjun commented on issue #10617: [Improvement][Master] Remove the extra check of slot

Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on issue #10617:
URL: https://github.com/apache/dolphinscheduler/issues/10617#issuecomment-1169505650

   > > > I have a different point, although we already have the database transaction to gurantee the command just be consumed once, but it's not a lightweight operations and will increase the database pressure, so I think the slot check, as the pre-check, is still necessary. WDYT
   > > 
   > > 
   > > We will check the slot when query from the database, so in most of the time, there will not database rollback. My concern is that is we use the `slotCheck` here, some user may think this method can guarantee the command be consumed safe by one master. So if we really don't want to remove these, we still need to add a comment to tell this method is not safe.
   > 
   > Yes, agree with you, we should add some comments for it.
   
   Ok, I will keep the slot check in out code, and just add some comment to tell this.


-- 
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@dolphinscheduler.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [dolphinscheduler] ruanwenjun closed issue #10617: [Improvement][Master] Remove the extra check of slot

Posted by GitBox <gi...@apache.org>.
ruanwenjun closed issue #10617: [Improvement][Master] Remove the extra check of slot
URL: https://github.com/apache/dolphinscheduler/issues/10617


-- 
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@dolphinscheduler.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [dolphinscheduler] github-actions[bot] commented on issue #10617: [Improvement][Master] Remove the extra check of slot

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on issue #10617:
URL: https://github.com/apache/dolphinscheduler/issues/10617#issuecomment-1166318398

   Thank you for your feedback, we have received your issue, Please wait patiently for a reply.
   * In order for us to understand your request as soon as possible, please provide detailed information、version or pictures.
   * If you haven't received a reply for a long time, you can [join our slack](https://s.apache.org/dolphinscheduler-slack) and send your question to channel `#troubleshooting`


-- 
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@dolphinscheduler.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org