You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/04/06 14:40:44 UTC

[GitHub] [airflow] kosteev opened a new pull request, #22784: Prevent breaking scheduler on task_instance.priority_weight column has overflow value

kosteev opened a new pull request, #22784:
URL: https://github.com/apache/airflow/pull/22784

   closes: #22781
   


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

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


[GitHub] [airflow] eladkal commented on pull request #22784: Prevent breaking scheduler on task_instance.priority_weight column has overflow value

Posted by GitBox <gi...@apache.org>.
eladkal commented on PR #22784:
URL: https://github.com/apache/airflow/pull/22784#issuecomment-1090381814

   The DAG shared on the issue runs just fine with Sqlite:
   ![Screen Shot 2022-04-06 at 17 55 52](https://user-images.githubusercontent.com/45845474/162004509-f89338db-9f73-469e-aea6-75115893ab24.png)
   
   ![Screen Shot 2022-04-06 at 17 55 41](https://user-images.githubusercontent.com/45845474/162004555-eeca6916-29ae-45c3-a281-e198d15436d9.png)
   
   
   I'm not sure if this is the right fix. Why should the model be concerned with issues originated from a specific DB?
   I suspect that this could be also reported on retries, pool_slots etc.
   I think the fix should be in the scheduler level(?)


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

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


[GitHub] [airflow] eladkal commented on pull request #22784: Prevent breaking scheduler on task_instance.priority_weight column has overflow value

Posted by GitBox <gi...@apache.org>.
eladkal commented on PR #22784:
URL: https://github.com/apache/airflow/pull/22784#issuecomment-1090584970

   I recall only https://github.com/apache/airflow/pull/17003 which also caused scheduler crash with DividedByZero


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

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


[GitHub] [airflow] kosteev closed pull request #22784: Prevent breaking scheduler on task_instance.priority_weight column has overflow value

Posted by GitBox <gi...@apache.org>.
kosteev closed pull request #22784: Prevent breaking scheduler on task_instance.priority_weight column has overflow value
URL: https://github.com/apache/airflow/pull/22784


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

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


[GitHub] [airflow] eladkal commented on pull request #22784: Prevent breaking scheduler on task_instance.priority_weight column has overflow value

Posted by GitBox <gi...@apache.org>.
eladkal commented on PR #22784:
URL: https://github.com/apache/airflow/pull/22784#issuecomment-1090570256

   > The fix is specific to Postgres I believe. Every database has different integer ranges so this will need to contain some if-else branches to cover each of them. Although honestly maybe it’s even better to just document this is a database limitation and you just can’t go over the limit.
   
   I agree but the point of concern is that this cause the scheduler to crash which is not good.
   It means that one dag can cause cluster wide problems. I think we need to find a way to locallize the effect so only the problematic dag will be effected.


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

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


[GitHub] [airflow] uranusjr commented on pull request #22784: Prevent breaking scheduler on task_instance.priority_weight column has overflow value

Posted by GitBox <gi...@apache.org>.
uranusjr commented on PR #22784:
URL: https://github.com/apache/airflow/pull/22784#issuecomment-1091763403

   Let’s start with adding just a check for Postgres now, we can add more later.


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

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


[GitHub] [airflow] kosteev commented on pull request #22784: Prevent breaking scheduler on task_instance.priority_weight column has overflow value

Posted by GitBox <gi...@apache.org>.
kosteev commented on PR #22784:
URL: https://github.com/apache/airflow/pull/22784#issuecomment-1091434178

   All right, validating it during DAG import and throwing error makes sense.
   What about different databases? Should we handle all the databases taking into account different constraints of each?


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

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


[GitHub] [airflow] ashb commented on pull request #22784: Prevent breaking scheduler on task_instance.priority_weight column has overflow value

Posted by GitBox <gi...@apache.org>.
ashb commented on PR #22784:
URL: https://github.com/apache/airflow/pull/22784#issuecomment-1090587743

   Is silently clamping it right, or should this be a parse time DAG import error?


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

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


[GitHub] [airflow] uranusjr commented on pull request #22784: Prevent breaking scheduler on task_instance.priority_weight column has overflow value

Posted by GitBox <gi...@apache.org>.
uranusjr commented on PR #22784:
URL: https://github.com/apache/airflow/pull/22784#issuecomment-1090571854

   Yeah the scheduler should not crash, but perhaps we could catch the exception or something like that. I seem to recall there’s a similar situation for something else a while ago (datetime out of range?)


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

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


[GitHub] [airflow] uranusjr commented on pull request #22784: Prevent breaking scheduler on task_instance.priority_weight column has overflow value

Posted by GitBox <gi...@apache.org>.
uranusjr commented on PR #22784:
URL: https://github.com/apache/airflow/pull/22784#issuecomment-1090555766

   The fix is specific to Postgres I believe. Every database has different integer ranges so this will need to contain some if-else branches to cover each of them. Although honestly maybe it’s even better to just document this is a database limitation and you just can’t go over the limit.


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

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


[GitHub] [airflow] uranusjr commented on pull request #22784: Prevent breaking scheduler on task_instance.priority_weight column has overflow value

Posted by GitBox <gi...@apache.org>.
uranusjr commented on PR #22784:
URL: https://github.com/apache/airflow/pull/22784#issuecomment-1090590207

   Ah found it, 2213635178ca9d0ae96f5f68c88da48f7f104bf1
   
   So I guess the better fix here is to do something similar, check for Postgres, and prevent an out-of-bounds value to be assigned for Postgres specifically.


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

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