move system outputs and analysis cases to Cloud Storage #335

lyuyangh · 2022-09-16T16:10:20Z

closes #308

System outputs and analysis cases are now stored in Cloud Storage so we can support datasets with large output files
- System outputs for CNN daily mail can be submitted successfully now but the analysis still takes a very long time.

Data schema of system_outputs_xxx collection
previous version:

_id: ObjectId # a system generated ID
system_id: str
analysis_level: str
data: BinData

new version:

_id: ObjectId # a system generated ID
system_id: str
analysis_level: str
data: str  # blob name (identifier for the object stored on Cloud Storage

Code is backward compatible so no DB migration is required

.env needs to be updated to include the bucket name and the GCP project name
Cloud Storage authentication:
- Local: authenticate with user accounts (will need to grant everyone access)
- ECS environment: authenticate with service accounts. The credentials are stored in AWS Secrets Manager and the credentials are passed in the containers as environment variables when the containers are initialized.

…g and prod

neubig

Looks nice!

Following this PR, maybe we could write a script that pulls all the files out of the MongoDB and writes them to cloud storage?

I approve personally, think @odashi definitely needs to take a look at this before it's merged though.

neubig · 2022-09-20T02:42:04Z

backend/src/impl/db_utils/system_db_utils.py

                insert_list = []
                sample_list = [general_to_dict(v) for v in system_output_data.samples]
-                sample_compressed = SystemDBUtils._compress_data(sample_list)
+
+                blob_name = f"{system_id}/{SystemDBUtils._SYSTEM_OUTPUT_CONST}"


I'm not sure how the GCP file system works, but would it be OK to, for example, have 1M files in one directory? If so then this is fine, but if not then we might want to have a hierarchical structure such as f"{system_id[0:3]}/{system_id}/{SystemDBUtils._SYSTEM_OUTPUT_CONST}"

I couldn't find a place in their doc where it explicitly says this but I think Cloud Storage (like S3) can basically hold any number of objects/files (S3 documentation says unlimited scalability). The only limit is that each object cannot exceed 5TB. So, I think this implementation is scalable.

re creating folders: Cloud Storage uses a flat namespace. If the object name contains "/"s, they are treated as part of the name. It does support folders but it basically creates an illusion for the user so it's easier to navigate the files with gsutil or on the web UI.

Cloud Storage uses a flat namespace.

Correct. Cloud Storage (and some other storage services) adopts flat ("no folders") structure and all allowed characters (including /) are treated as a part of file name.

Cloud storage can store basically unlimited number of files as long as the data center allows. The only concern is the increase of cost of managing the data.

neubig · 2022-09-20T02:43:16Z

backend/src/impl/db_utils/system_db_utils.py

+            )
+        data = next(cursor)["data"]
+        # NOTE: (backward compatibility) data was stored in MongoDB previously,
+        # this can be removed after we regenerate DB next time.


Explaining what "this" is would make this more clear.

I have updated the comment.

lyuyangh · 2022-09-20T03:56:23Z

Thanks, @neubig! I implemented the code in a backward compatible way because I think it'll be easier to handle it in the code than migrate the DB. If we decide to migrate the DB with a script, I'll remove the code that's added for compatibility reasons.

odashi · 2022-09-20T06:59:13Z

@lyuyangh Could you prepare the plan of DB migration in some documents (or issues)? It is useful to understand how/when backward-incompatible changes will happen on this repository.

odashi · 2022-09-20T07:48:18Z

README.md

@@ -6,7 +6,7 @@ This repository includes code for frontend and backend of the ExplainaBoard web

 [Schema Design](https://docs.google.com/document/d/1my-zuIYosrXuoqOk1SvqZDsC2tdMgs_A7HTAtPYXKLI/edit?usp=sharing)

-## Quick Start
+## Quick Start for Developers


Does the "developer" here mean the developers of this software?
If it means any developers, it may involve some security risks (especially granting access to GCS) and I couldn't allow them to give authorization.

Yes, developers of explainaboard_web. I don't think outside developers can actually contribute to this codebase at this point. As you mentioned, it's not secure to give them access to GCP and AWS so they'll need to build their own infrastructure (Cloud Storage, Cognito, MongoDB). If we want outside contributors, I guess we need a way for them to easily mock or set up local instances to replace these services.

Agreed, thanks!

backend/src/impl/storage.py

backend/src/impl/init.py

Co-authored-by: Yusuke Oda <[email protected]>

lyuyangh · 2022-09-20T14:37:42Z

@odashi Thanks for the comments! I have fixed all of them.

We don't need to do a DB migration for this PR (it is backward compatible). But I agree, it'll be helpful to keep track of all the backward incompatible changes. We have #336 but it's specifically for breaking changes related to SDK v0.11.2. It will probably be better if we have a document that we can keep adding things to. Shall I create a Google doc for this?

odashi · 2022-09-20T14:44:58Z

@lyuyangh I think creating an issue on this repository focusing on DB migration is enough at this point.

odashi

Thanks!

lyuyangh requested review from neubig and odashi September 16, 2022 16:14

lyuyangh marked this pull request as draft September 16, 2022 16:46

lyuyangh force-pushed the google-cloud branch from fce86fd to 4dc324e Compare September 19, 2022 14:10

move system outputs and analysis cases to Cloud Storage

873bf71

lyuyangh force-pushed the google-cloud branch 2 times, most recently from 4520585 to 7138a0d Compare September 20, 2022 00:54

lyuyangh marked this pull request as ready for review September 20, 2022 01:02

use secrets manager to distribute Cloud Storage credentials in stagin…

2a86d65

…g and prod

lyuyangh force-pushed the google-cloud branch from 7138a0d to 2a86d65 Compare September 20, 2022 01:04

neubig reviewed Sep 20, 2022

View reviewed changes

better comment about backward compatibility

4075599

odashi reviewed Sep 20, 2022

View reviewed changes

lyuyangh and others added 2 commits September 20, 2022 09:28

Apply suggestions from code review

cf1a4eb

Co-authored-by: Yusuke Oda <[email protected]>

load gcp credentials with google.oauth2

b344125

lyuyangh force-pushed the google-cloud branch from 2b33fd9 to b344125 Compare September 20, 2022 14:26

odashi approved these changes Sep 20, 2022

View reviewed changes

lyuyangh mentioned this pull request Sep 20, 2022

DB Migrations and Backward Compatibility Issues #345

Open

lyuyangh merged commit 01b0fb4 into main Sep 20, 2022

lyuyangh deleted the google-cloud branch September 20, 2022 16:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

move system outputs and analysis cases to Cloud Storage #335

move system outputs and analysis cases to Cloud Storage #335

lyuyangh commented Sep 16, 2022 •

edited

Loading

neubig left a comment

neubig Sep 20, 2022

lyuyangh Sep 20, 2022 •

edited

Loading

odashi Sep 20, 2022

neubig Sep 20, 2022

lyuyangh Sep 20, 2022

lyuyangh commented Sep 20, 2022

odashi commented Sep 20, 2022

odashi Sep 20, 2022

lyuyangh Sep 20, 2022

odashi Sep 20, 2022

lyuyangh commented Sep 20, 2022 •

edited

Loading

odashi commented Sep 20, 2022

odashi left a comment

move system outputs and analysis cases to Cloud Storage #335

move system outputs and analysis cases to Cloud Storage #335

Conversation

lyuyangh commented Sep 16, 2022 • edited Loading

neubig left a comment

Choose a reason for hiding this comment

neubig Sep 20, 2022

Choose a reason for hiding this comment

lyuyangh Sep 20, 2022 • edited Loading

Choose a reason for hiding this comment

odashi Sep 20, 2022

Choose a reason for hiding this comment

neubig Sep 20, 2022

Choose a reason for hiding this comment

lyuyangh Sep 20, 2022

Choose a reason for hiding this comment

lyuyangh commented Sep 20, 2022

odashi commented Sep 20, 2022

odashi Sep 20, 2022

Choose a reason for hiding this comment

lyuyangh Sep 20, 2022

Choose a reason for hiding this comment

odashi Sep 20, 2022

Choose a reason for hiding this comment

lyuyangh commented Sep 20, 2022 • edited Loading

odashi commented Sep 20, 2022

odashi left a comment

Choose a reason for hiding this comment

lyuyangh commented Sep 16, 2022 •

edited

Loading

lyuyangh Sep 20, 2022 •

edited

Loading

lyuyangh commented Sep 20, 2022 •

edited

Loading