Tfx: "Type already exists with different properties error" when running TFX 0.21.2 and TFX 0.23.0 in same kubeflow

[pselden]

One of the users of my kubeflow cluster is using TFX 0.21.4 and another is using TFX 0.23.0

After any pipeline is run that uses TFX 0.23.0, the ones that use TFX 0.21.4 fail with the following error during BigQueryExampleGen

Traceback (most recent call last):
  File "/tfx-src/tfx/orchestration/kubeflow/container_entrypoint.py", line 382, in <module>
    main()
  File "/tfx-src/tfx/orchestration/kubeflow/container_entrypoint.py", line 375, in main
    execution_info = launcher.launch()
  File "/tfx-src/tfx/orchestration/launcher/base_component_launcher.py", line 197, in launch
    self._exec_properties)
  File "/tfx-src/tfx/orchestration/launcher/base_component_launcher.py", line 166, in _run_driver
    component_info=self._component_info)
  File "/tfx-src/tfx/components/base/base_driver.py", line 289, in pre_execution
    contexts=contexts)
  File "/tfx-src/tfx/orchestration/metadata.py", line 601, in update_execution
    registered_artifacts_ids=registered_output_artifact_ids))
  File "/tfx-src/tfx/orchestration/metadata.py", line 538, in _artifact_and_event_pairs
    a.set_mlmd_artifact_type(self._prepare_artifact_type(a.artifact_type))
  File "/tfx-src/tfx/orchestration/metadata.py", line 184, in _prepare_artifact_type
    artifact_type=artifact_type, can_add_fields=True)
  File "/opt/venv/lib/python3.6/site-packages/ml_metadata/metadata_store/metadata_store.py", line 268, in put_artifact_type
    self._call('PutArtifactType', request, response)
  File "/opt/venv/lib/python3.6/site-packages/ml_metadata/metadata_store/metadata_store.py", line 131, in _call
    return self._call_method(method_name, request, response)
  File "/opt/venv/lib/python3.6/site-packages/ml_metadata/metadata_store/metadata_store.py", line 162, in _call_method
    raise _make_exception(e.details(), e.code().value[0])
tensorflow.python.framework.errors_impl.AlreadyExistsError: Type already exists with different properties.

I completely wiped out my kubeflow cluster/metadata to test from scratch, and the same thing happens. Is there any way to avoid that short of forcing everyone to be on the same tfx version?

[pselden]

Anyone else having this issue or know a workaround?

[calvinleungyk]

This happens with 0.22.1 and 0.24.0 as well. Throwing an error using tfx.extensions.google_cloud_big_query.example_gen.component import BigQueryExampleGen

[casassg]

Running into same issue with upgrade, we are trying to debug to identify possible workarounds

[1025KB]

This might due to artifact's properties changes between TFX versions, the compatibility is single direction.
If Metadata type is updated, the old type might become incompatible.

Consider using different metadata db when run old version of TFX?

[pselden]

Is that an “expected” incompatibility that can be introduced in future versions? It seems acceptable although annoying that within a single pipeline the upgrade is a one-way process but for it to affect completely unrelated pipelines is quite bad.

How would I go about creating and using a parallel metadata DB?

[1025KB]

In our current TFX examples, each pipeline has its own Metadata DB.

+Hui@, Do you know if there is any pipeline level isolation provided in MLMD recently?

[pselden]

How is that supposed to work with Kubeflow though? There's a single metadata service that connects to a single metadata database.

[casassg]

Would be nice to have this actually be directional. On larger TFX deployments, you may want to reuse MLMD instances. Also there's potentialy to use centralized MLMD to reuse artifacts across pipelines using ResolverNodes. But this wont be possible if every time there's an upgrade it's not backwards compatible as code can't be redeployed for every single pipeline.

[zhitaoli]

I think I agree with Paul and Gerard that we should aim to get support of
multi-tenanted MLMD working. We had some existing designs but had not got
time to prioritize implementation.

@Charles Chen [e-mail] @Hui Miao [e-mail] @Neoklis
Polyzotis [e-mail] can help to comment on readiness of
related proposals.

On Mon, Oct 5, 2020 at 2:37 PM Gerard Casas Saez [e-mail]
wrote:

Would be nice to have this actually be directional. On larger TFX
deployments, you may want to reuse MLMD instances. Also there's potentialy
to use centralized MLMD to reuse artifacts across pipelines using
ResolverNodes. But this wont be possible if every time there's an upgrade
it's not backwards compatible as code can't be redeployed for every single
pipeline.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/tensorflow/tfx/issues/2506#issuecomment-703902937,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAY6AZWS4JW2FDMFTGZR6UDSJI4CJANCNFSM4RQ7YWCA
.

--
Cheers,

Zhitao Li

[casassg]

On a different note, is there any chance we can add some testing to avoid this type of breaking update from happening again? 

It was surprising to hear that backwards compatibility on MLMD schema is not granted. We expected that if MLMD had the same schema then we wouldn't have issues on upgrade or downgrade. We did this upgrade blinded by the fact MLMD repo said the schema was the same version as the previous version. 

In addition, we are lacking tools to perform a downgrade of TFX types to the previous version which is making our job at the current moment quite hard.

We are adding more processes to perform these upgrades on our side to testing that: Running our example pipeline on a new version of TFX and then running the same pipeline with the existing version of TFX in a shared MLMD works.

[1025KB]

This might due to artifact's properties changes between TFX versions, the compatibility is single direction.
If Metadata type is updated, the old type might become incompatible.

To just clarify this comments, this issue happens when the properties set of the type in mlmd is not a super or subset of the new properties set.

E.g., in metadata, the properties of type X is (A,B,C), register new superset (A,B,C,D) or subset(A,B) will be compatible. But error happens when trying to register type X with properties like (A,B,D)

[casassg]

Yup, I understand. I just don't understand why using BigQueryExampleGen on 0.22.1 and 0.24.0 can't happen at the same time.

After debugging a bit more though, I think we can narrow down a bit more using the GRPC server error log

2020-10-06 00:26:53.508991: W ml_metadata/metadata_store/metadata_store_service_impl.cc:59] PutArtifactType failed: Type already exists with different properties.

Seems the issue is with an artifact type. Could it be due to Example standard artifact adding new properties? Given that it hasn't removed any property I think it should work as it's a superset (upgrade version) and then a subset (downgrade). Unless I'm missing something

I've been looking at https://github.com/tensorflow/tfx/compare/v0.22.1...v0.24.0 to see if there's any of the (A,B,D) situation you described.

Btw, another action item would be to make those errors a bit more verbose. It would be nice to have TFX tell us what type is the one that failed to register.

[1025KB]

one way to find out is add some logs in mlmd to print detailed information like this

Here is the call for artifact type

(or just query MLMD to check the type information)

we did add new properties to the artifacts. But adding new properties should work.

[casassg]

How about removing them? The issue happens after you run a pipeline in 0.24 (which adds the properties to the artifact type) and then running a pipeline in 0.22.1 (which tries to remove the properties?)

[casassg]

Okay, after checking https://github.com/tensorflow/tfx/blob/master/tfx/orchestration/metadata.py#L185 and https://github.com/google/ml-metadata/blob/59c6afc76a339536600f782b9c4578a662cc6bd2/ml_metadata/metadata_store/metadata_store.py#L232 I'm guessing (hunch) that the issue may be due to TFX calling MLMD using put_artifact_type(..., all_fields_match = True) when registering the artifact. This may make the call to fail when there are fields registered in MLMD but are not existing in the current representation.

I'm going to try to recreate the issue as a unit test and see if it gets solved by setting all_fields_match = False.

[1025KB]

@hui, what happens when we set both can_add_fields and all_fields_match(default) to true

[casassg]

I was able to reproduce error with a simple test: https://github.com/tensorflow/tfx/pull/2606

Working to file a fix now that the issue is more isolated.

[casassg]

Seems fix is not possible due to option not being implemented in MLMD: https://github.com/google/ml-metadata/blob/1fceefd596e2ed7e98e629e7186e0f0b1fbace28/ml_metadata/metadata_store/metadata_store.cc#L248

[casassg]

Here's the culprit: https://github.com/google/ml-metadata/blob/1fceefd596e2ed7e98e629e7186e0f0b1fbace28/ml_metadata/metadata_store/metadata_store.cc#L35 Seems it only allows to add fields. If registered fields are a superset of artifact then it will fail.

So:

• (AB) -> (ABC) - Works!
• (ABC) -> (AB) - Doesn't work!
• (ABC) -> (ABD) - Doesn't work!

[charlesccychen]

CC: @hughmiao

[hughmiao]

From MLMD, as the instances (e.g., artifacts) of old types are stored, when updating users' type, its properties must be a superset of the properties of the stored type schema. Currently we do not allow to delete properties or change existing property's value type to ensure the data backward compatibility. (i.e., the new type schema can be used to parse the stored instances).

To support type evolution properly, some discussions and caveats had in the past, e.g.,

a) In MLMD, to allow the same types have versioned identifiers, so that they don't conflict (as there are multiple copies of the type schema stored) and we infer the compatibilities. This may be tricky for applications (TFX) for querying stored information, e.g., imaging building a cache resolution logic on the v0.21, v0.23, v0.24 runs, shall it respect the older tfx run or not, etc. For example, when building a UI, does it need to be aware of the differences of the type versions.

AND / OR

b) in TFX, it makes sure that the type evolutions can co-exist in two ways: older tfx can work with newer tfx types, and newer tfx can read older tfx instances. For example, here, when v0.21 saw 0.23 types, which is a superset, it considers it as future change is backward compatible, etc. Without an explicit rule, the tricky part is v0.21 does not possibly know that what kind of 0.23's future change will be and whether the superset is safe to proceed. For 0.23 to read 0.21, it will be good to capture the deprecated types, renamed types etc.

As you can see, supporting this can go quite complicated. /cc @charlesccychen to add other comments.

I think I agree with Paul and Gerard that we should aim to get support of multi-tenanted MLMD working.

+1 to zhitaoli@'s comment that we should prioritize this and better support multi-tenanted MLMD instances.

For now, as the 0.21.0 is released, to unblock the case, the shared mlmd instance must have compatible application level types. When upgrading tfx, please consider to use a new db to partition the different pipelines based on their tfx versions.

For the short term solution, in the new release, supporting/using all_fields_match = True, which is quite tricky and undeterministic depending on the order of the type registration call. Another thought is in tfx metadata.py always trying to explicitly read the stored artifact type of the same name and by pass the registration if it is a superset.

/cc @ruoyu90 to comment more.

[casassg]

Thanks for your thoughtful response!

Would love to have an RFC or a similar discussion topic around issue exposed in a) and b). Not sure what's the best location for that though.

We do have a system internally that allows us partition and have 2 version at the same time at this point. I guess my main issue was that this was not easy to debug (I think we can add documentation to error that is easier to understand) and we were caught by surprise as it was not surfaced in MLMD documentation or release notes (or it was not clear that it was there). We are adding internal safeguards to avoid this to happen again and rolling 0.24 to most pipelines. It's a bit difficult since any pipeline that runs 0.24 then can immediatly make the DB unusable for 0.22.1.

Also, did you mean all_fields_match = False? And TFX layer solution also works imo. I can propose that one in the PR I linked here.

[hughmiao]

did you mean all_fields_match = False? And TFX layer solution also works imo.

Gerard: all_fields_match = False semantics is tricky when mixing tfx versions pipelines and register types concurrently. The behavior is undeterministic, i.e., 0.21 may overwrite 0.23, and vise versa.

We probably needs a semantics of succeed_if_compatible, for the short term, my suggestion was putting the skipping logic (stored type is compatible) explicitly and skip the registration (call put_artifact_type) in the tfx layer when calling put_artifact_type.

/cc @ruoyu90 @charlesccychen

[casassg]

Fair, succeed_if_compatible sounds good to me. I saw all_field_match as an equivalent to succeed_if_subset which is why I proposed this.

Regarding what layer, I think adding it on MLMD is better long term (as more users may be able to benefit from this. for ex: we are actually looking into using MLMD to track more generic jobs and training, even outside of TFX). But not sure how easy to fix/timeframe would be for each.

[hughmiao]

Implementing a succeed_if_compatible semantics can be done easily in tfx, i.e., catching the AlreadyExists error, and reuse the type_id in [1]. 0.23 will always succeed, while 0.21 goes to the fallback branch to reuse the new type.

If this is a blocking issue, either change that line and use a locally built wheel or we can do a patch release to make the change to work for this case 0.21 with 0.23. wdyt?

Agree that it will be good to have this semantics in mlmd to let it be used others in the long term, which requires a mlmd release, and change tfx to use that mode. I will work on this and update the usage of tfx at head for the next release.

[1] https://github.com/tensorflow/tfx/blob/7c4f240f888c9b0aa92ba8e1cc07dc08c9c5f687/tfx/orchestration/metadata.py#L183

[pselden]

If this is a blocking issue, either change that line and use a locally built wheel or we can do a patch release to make the change to work for this case 0.21 with 0.23. wdyt?

A patch release for 0.21.x would be awesome and would solve my immediate problem.

[casassg]

For us a fix it forward works (aka fix it in MLMD for next release). As @pselden mentions, it would be nice to have the patch release as well in case we need to revert pipelines to 0.22 as we continue migration to 0.23/0.24.

[hughmiao]

sg. we are working on this and will do patch release for 0.21.x and 0.22.x.

[pselden]

This is working now.

[tensorflow-butler[bot]]

Are you satisfied with the resolution of your issue?
Yes
No