-
Notifications
You must be signed in to change notification settings - Fork 14.3k
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
Remove XCom pickling #43905
Remove XCom pickling #43905
Conversation
39d2306
to
fd62961
Compare
fd62961
to
e6cdc10
Compare
e6cdc10
to
f57006e
Compare
If JSON decoding fails, maybe we could save the value in a backup blob field (to be deprecated and removed in a specified future version). While XComs are generally not that important I feel it might be better to soft-delete wherever there's a risk of data loss as a general practice. |
I like the drop of the pickle type - but I'd favor not keeping this as blob/binary in the DB. Then we can not use any DB feature to efficiently "use" the data other than a blob. If not in this PR, can we have a follow-up that converts the data into (MySQL is: https://dev.mysql.com/doc/refman/8.4/en/json.html) And totally forgot about the vote: I think we can also consider to provide an offline migration tool which we request to be executed by everybody manually prior upgrade such that we don't need to carry complex inline migration. Everybody who uses XCom pickling should know this from the config. So Option 1 or Option 1a (a=offline tool) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
f57006e
to
da0e059
Compare
I will create a separate PR to handle the migration so that can be reviewed independently -- will have a PR by EOD today |
da0e059
to
234406b
Compare
XCom pickling was disabled by default in Airflow 2.0.0: https://airflow.apache.org/docs/apache-airflow/1.10.15/configurations-ref.html#enable-xcom-pickling
234406b
to
fc0c60b
Compare
PR created: #44166 |
XCom pickling was disabled by default in Airflow 2.0.0: https://airflow.apache.org/docs/apache-airflow/1.10.15/configurations-ref.html#enable-xcom-pickling
follow-up of #43905 Changes: - Changed `XCom.value` column to JSON for all dbs. - Archived pickled XCom data to `_xcom_archive` and removed it from the `xcom` table. - Removed encoded string in XCom serialization and deserialization logic. - Updated logic for `XComObjectStorageBackend` to make it compatible for AF 2 & 3
XCom pickling was disabled by default in Airflow 2.0.0: https://airflow.apache.org/docs/apache-airflow/1.10.15/configurations-ref.html#enable-xcom-pickling
Discussion
To avoid a time-consuming DB migration, I have not changed the column type of
value
; it is stillLargeBinary
/LONGBLOB
(MySQL).As part of Airflow 3, we should strongly recommend users to use the
airflow db clean
command to prune the DBs to the minimum required. If we assume, most users would do that, we can run the following migration:Option 1: Try to migrate pickle to JSON
Option 2: Delete XCom rows with pickle
Option 3: Keep the current column type
Not optimal, but we can keep the current column type to binary/long-blob
Any thoughts?
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named
{pr_number}.significant.rst
or{issue_number}.significant.rst
, in newsfragments.