New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 705002 link

Starred by 1 user
Status: Fixed
Owner:
no...@chromium.org
Closed: Apr 2017
Cc:
█ iannucci@chromium.org
d...@chromium.org
no...@chromium.org
kjlubick@chromium.org
vadimsh@chromium.org
sergeybe...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Ingestion of textprotos by components/config/ should store intermediary format in binary encoded protos

Project Member Reported by mar...@chromium.org, Mar 24 2017 
We have a somewhat frequent source of downtime where:
1) Someone adds a new field in a luci-py GAE server proto
2) The person deploys everywhere
3) The person start using the new field in luci-config managed repo

The problem is that old versions could still be running, which is particularly true on Swarming as the bot version-lock the server for the duration of a task.

The current options are:
- Wait several hours between 2) and 3).
- Accept some HTTP 500.

Since there's no signal if old server version instances are still running or not, people occasionally start using the new fields too quickly.

A long term fix is:
- Upon ingestion, the textproto is convert to binary encoded proto.
- The binary encoded data is used as the canonical local cache.

Comment 1 by d...@chromium.org, Mar 24 2017 

FTR this is implemented in the Go "luci_config" caching layers: https://godoc.org/github.com/luci/luci-go/luci_config/server/cfgclient/textproto

It doesn't solve the problem of old instance versions doing the caching, but I think that's less of an issue.

Comment 2 by no...@chromium.org, Mar 24 2017

Owner: no...@chromium.org
Status: Started (was: Untriaged)

Comment 3 by mar...@chromium.org, Mar 29 2017

Cc: kjlubick@chromium.org sergeybe...@chromium.org
 Issue 638245  has been merged into this issue.
Project Member

Comment 4 by bugdroid1@chromium.org, Mar 29 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/external/github.com/luci/luci-py.git/+/b048c2783c11ff79dfc32f3925a65d1f42828ac7

commit b048c2783c11ff79dfc32f3925a65d1f42828ac7
Author: nodir <nodir@chromium.org>
Date: Wed Mar 29 22:27:16 2017

config: store binary configs

config.get_async: if last good config is requested and destination type
is a protobuf message, store it in binary in addition to raw format.

When loading from a last good conig, if protobuf message type name
matches what we have in the datastore, load the config from the binary
field.

Also fix tests and include them in PRESUBMIT.py

R=vadimsh@chromium.org
BUG= 705002 

Review-Url: https://codereview.chromium.org/2778533002

[modify] https://crrev.com/b048c2783c11ff79dfc32f3925a65d1f42828ac7/appengine/components/PRESUBMIT.py
[modify] https://crrev.com/b048c2783c11ff79dfc32f3925a65d1f42828ac7/appengine/components/components/config/api.py
[modify] https://crrev.com/b048c2783c11ff79dfc32f3925a65d1f42828ac7/appengine/components/components/config/api_test.py
[modify] https://crrev.com/b048c2783c11ff79dfc32f3925a65d1f42828ac7/appengine/components/components/config/fs.py
[modify] https://crrev.com/b048c2783c11ff79dfc32f3925a65d1f42828ac7/appengine/components/components/config/proto/project_config_pb2.py
[modify] https://crrev.com/b048c2783c11ff79dfc32f3925a65d1f42828ac7/appengine/components/components/config/proto/service_config_pb2.py
[modify] https://crrev.com/b048c2783c11ff79dfc32f3925a65d1f42828ac7/appengine/components/components/config/remote.py
[modify] https://crrev.com/b048c2783c11ff79dfc32f3925a65d1f42828ac7/appengine/components/components/config/remote_test.py
[modify] https://crrev.com/b048c2783c11ff79dfc32f3925a65d1f42828ac7/appengine/components/components/config/test_config_pb2.py

Comment 5 by no...@chromium.org, Apr 5 2017

Status: Fixed (was: Started)
still requires verification
►

Sign in to add a comment