New issue
Advanced search Search tips

Issue 923324 link

Starred by 1 user

Issue metadata

Status: Untriaged
Owner: ----
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

bqschemaupdater: promote google.protobuf.Duration to FLOAT64

Project Member Reported by mar...@chromium.org, Jan 18 (4 days ago)

Issue description

Background:
bqschemaupdater has explicit knowledge for google.protobuf.Timestamp and converts it to BigQuery native type TIMESTAMP. On the other hand, google.protobuf.Duration is not special cased, and since it's a protobuf message, it's converted into a RECORD(seconds, nanos).

Issue:
It is problematic to work with RECORD(seconds, nanos) do to any kind of calculation. This comes up in particular with Swarming because I want to calculate efficiency, so I'm left with 3 choices:
1. Drop subsecond resolution for simplicity
2. Write super complex queries
3. Update bqschemaupdater to convert google.protobuf.Duration to a native BQ type FLOAT64 that is easier to work with

This issue is about implementing 3.

AIs:
- Update bqschemaupdater to convert google.protobuf.Duration to FLOAT64
- Update go.chromium.org/luci/common/bq
- Update infra_libs/bqh.py
- Deploy the new schema
- Deploy the new server code

This is potentially risky for some services, but I don't know the extent of how many BigQuery tables there exist, and I think the upgrade has to be done transactionally and with data loss: rows will be refused, hence we likely are going to have to completely drop the tables and start over. This is fine for Swarming right now, but likely not for other services.

Refs:
- https://cloud.google.com/bigquery/docs/reference/standard-sql/data-types
- https://developers.google.com/protocol-buffers/docs/reference/csharp/class/google/protobuf/well-known-types/duration
- https://chromium.googlesource.com/infra/luci/luci-go.git/+/fc59bd114abd60c52ccad825779d9e1f2406fd5d/tools/cmd/bqschemaupdater/schema.go#112
- https://chromium.googlesource.com/infra/luci/luci-go.git/+/fc59bd114abd60c52ccad825779d9e1f2406fd5d/common/bq/eventupload.go#223
- https://chromium.googlesource.com/infra/infra.git/+/ef204aed33f2cdf95b5d71e46642bb9701492bec/packages/infra_libs/infra_libs/bqh.py#71
 
Project Member

Comment 1 by bugdroid1@chromium.org, Yesterday (37 hours ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-go.git/+/0717e8c2c7fb3ee7136f63691808b348e69b4bf6

commit 0717e8c2c7fb3ee7136f63691808b348e69b4bf6
Author: Marc-Antoine Ruel <maruel@chromium.org>
Date: Mon Jan 21 16:35:20 2019

[bq] [bqschemaupdater] Convert google.protobuf.Duration to FLOAT64

**This is a disruptive change**

- When creating a BQ schema, use FLOAT64 for google.protobuf.Duration
  instead of expanding as RECORD(seconds, nanos).
- When uploading, use a float.

When a user updates the schema, the user must also update the server
code immediately.

Bug: 923324
Change-Id: I84bfc5bc9666b558b6cfab4e2b90014699169fe5
Reviewed-on: https://chromium-review.googlesource.com/c/1421478
Commit-Queue: Marc-Antoine Ruel <maruel@chromium.org>
Reviewed-by: Nodir Turakulov <nodir@chromium.org>

[modify] https://crrev.com/0717e8c2c7fb3ee7136f63691808b348e69b4bf6/common/bq/eventupload.go
[modify] https://crrev.com/0717e8c2c7fb3ee7136f63691808b348e69b4bf6/common/bq/eventupload_test.go
[modify] https://crrev.com/0717e8c2c7fb3ee7136f63691808b348e69b4bf6/common/bq/testdata/testmessage.pb.go
[modify] https://crrev.com/0717e8c2c7fb3ee7136f63691808b348e69b4bf6/common/bq/testdata/testmessage.proto
[modify] https://crrev.com/0717e8c2c7fb3ee7136f63691808b348e69b4bf6/tools/cmd/bqschemaupdater/doc.go
[modify] https://crrev.com/0717e8c2c7fb3ee7136f63691808b348e69b4bf6/tools/cmd/bqschemaupdater/main_test.go
[modify] https://crrev.com/0717e8c2c7fb3ee7136f63691808b348e69b4bf6/tools/cmd/bqschemaupdater/schema.go
[modify] https://crrev.com/0717e8c2c7fb3ee7136f63691808b348e69b4bf6/tools/cmd/bqschemaupdater/testdata/event.desc
[modify] https://crrev.com/0717e8c2c7fb3ee7136f63691808b348e69b4bf6/tools/cmd/bqschemaupdater/testdata/event.pb.go
[modify] https://crrev.com/0717e8c2c7fb3ee7136f63691808b348e69b4bf6/tools/cmd/bqschemaupdater/testdata/event.proto

Project Member

Comment 2 by bugdroid1@chromium.org, Yesterday (37 hours ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra/+/64c25557a02cfd4781e64b757c4145cbb3eafec3

commit 64c25557a02cfd4781e64b757c4145cbb3eafec3
Author: Marc-Antoine Ruel <maruel@chromium.org>
Date: Mon Jan 21 16:38:02 2019

[infra_libs] Update bqh to handle google.protobuf.Duration as FLOAT64

This is a disruptive change, see corresponding bqschemaupdater change in
go.chromium.org/luci at https://chromium-review.googlesource.com/1421478

Bug: 923324
Change-Id: Icdfeebff07abcbf42f89e8822d19d300ccfa8499
Reviewed-on: https://chromium-review.googlesource.com/c/1421479
Commit-Queue: Marc-Antoine Ruel <maruel@chromium.org>
Reviewed-by: Nodir Turakulov <nodir@chromium.org>
Cr-Commit-Position: refs/heads/master@{#20100}
[modify] https://crrev.com/64c25557a02cfd4781e64b757c4145cbb3eafec3/packages/infra_libs/infra_libs/test/testmessage_pb2.py
[modify] https://crrev.com/64c25557a02cfd4781e64b757c4145cbb3eafec3/packages/infra_libs/infra_libs/bqh.py
[modify] https://crrev.com/64c25557a02cfd4781e64b757c4145cbb3eafec3/packages/infra_libs/infra_libs/test/testmessage.proto
[modify] https://crrev.com/64c25557a02cfd4781e64b757c4145cbb3eafec3/packages/infra_libs/infra_libs/test/bqh_test.py

Project Member

Comment 3 by bugdroid1@chromium.org, Yesterday (35 hours ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-py.git/+/9647885b80a223b42262e9f0d73e9fdf083dd346

commit 9647885b80a223b42262e9f0d73e9fdf083dd346
Author: Marc-Antoine Ruel <maruel@chromium.org>
Date: Mon Jan 21 19:02:09 2019

[infra_libs] Update bqh.py to infra_libs v2.0.0

TBR'ed because I want to push this live to the services while the US is on an
holiday, as this CL requires recreating the BigQuery tables, and to make it
viable, I need to disable the cron jobs temporarily.

TBR=qyearsley@chromium.org

Bug: 765435, 923324
Change-Id: Iba2a6b70cbd73534b91a7bdfa77da2a49342e209
Reviewed-on: https://chromium-review.googlesource.com/c/1425777
Reviewed-by: Marc-Antoine Ruel <maruel@chromium.org>
Commit-Queue: Marc-Antoine Ruel <maruel@chromium.org>

[modify] https://crrev.com/9647885b80a223b42262e9f0d73e9fdf083dd346/client/third_party/infra_libs/README.swarming
[modify] https://crrev.com/9647885b80a223b42262e9f0d73e9fdf083dd346/client/third_party/infra_libs/bqh.py

Sign in to add a comment