-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Drop Metadata table during API startup time #1728
Comments
We have already started using mlmetadata database for our own tfx components. I wonder if kfp can offer a more sophisticated db migration plan instead of simply dropping and recreating the database. Currently implementation proposed in #1729 will completely break our tfx components. Thanks! |
Hi @daikeshi we're planning to deprecate the MLMD database within API server, and replace it with Kubeflow Metadata server instead. MLMD itself is unable to migrate just yet (though that's a feature that's coming soon AFAIK). In the meantime, I can change this PR to do one of the following:
I'm open to other suggestions as well. /hold |
Thanks @neuromage! I think we would favor option 2 (dropping DB if only a flag is set). At least, this approach will not delete data silently when we upgrade the kfp version. Another thought I have is: api server can also check tables in the DB, if they are all empty, it's safe to drop the database, ow, throw an error unless user sets a flag to force to do it. |
Thanks @daikeshi. I've put this behind a flag now. I didn't do the check tables bit, since the schema is an implementation detail. Also, we do plan to move this out of APIServer soon. PTAL and let me know what you think. |
@neuromage your change looks good. I believe we will be able to set the flag somewhere in pipeline deployment manifest? Also what's the timeline to remove MLMD in KFP API server? It will also work for us if KFP can just ignore existing mlmetadata database in MySQL. |
@daikeshi yes, when we remove it, we'll remove the code, and leave existing dbs alone. The timeline is this quarter, probably by end of Aug. By then, MLMD will also be able to upgrade the schema by itself. You should be able to set the flag in the pipeline deployment manifest. |
any update? |
We've removed this from the API server, so new versions should no longer talk to the MLMD DB in the backend. I think we can close this now. @daikeshi please let us know if this is acceptable and/or if you have any more issues around this. |
The API server starting to crash after upgrade from latest release 0.1.25 to HEAD 44f8198 potentially caused by this
#1725
Due to incompatible DB schema changes
F0803 19:07:07.466998 9 client_manager.go:179] Failed to create ML Metadata store: mysql_query failed: errno: 1054, error: Unknown column 'input_type' in 'field list'
goroutine 1 [running]:
github.com/golang/glog.stacks(0xc0002f9300, 0xc00093e000, 0xad, 0x1ad)
external/com_github_golang_glog/glog.go:769 +0xd4
github.com/golang/glog.(*loggingT).output(0x2b25820, 0xc000000003, 0xc0005791e0, 0x29e9cfc, 0x11, 0xb3, 0x0)
external/com_github_golang_glog/glog.go:720 +0x329
github.com/golang/glog.(*loggingT).printf(0x2b25820, 0xc000000003, 0x1bdc12a, 0x26, 0xc0006efbd0, 0x1, 0x1)
external/com_github_golang_glog/glog.go:655 +0x14b
github.com/golang/glog.Fatalf(0x1bdc12a, 0x26, 0xc0006efbd0, 0x1, 0x1)
external/com_github_golang_glog/glog.go:1148 +0x67
main.initMetadataStore(0xc0000443ee)
backend/src/apiserver/client_manager.go:179 +0x31f
main.(*ClientManager).init(0xc0006efcd8)
backend/src/apiserver/client_manager.go:148 +0x52f
main.newClientManager(0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
backend/src/apiserver/client_manager.go:301 +0x7b
main.main()
backend/src/apiserver/main.go:56 +0x5e
Since we don't have guarantee on MLMD schema, we could drop the database when API starts up .
The text was updated successfully, but these errors were encountered: