LogDog: Update to archival V2.

LogDog: Update to archival V2.

Remove archive task polling. The standard path to archival will be archive
tasks created when the stream is registered as terminated. These tasks
will stick around until the archival is successful or has been declared
a failure.

Archival completeness is now a property of its task: if the task is
older than a threshold, the archive's completeness requirement will be
dropped.

Archival also offers an error path which will archive an empty stream if
there is a fatal (non-transient) archival error. This primarily targets
data errors.

An emergency poller will still run periodically to catch streams that
have not been terminated, either because of data loss or because the
Butler never sent a terminal index (i.e., it crashed).

We also switch from pull queues (alpha REST API) to Pub/Sub for archival
tasking. A unique key is sent with each Pub/Sub task to recreate
transactional task queue operations in Pub/Sub - only the Pub/Sub message
that includes the key transactionally encoded into Datastore will be
used for archival.

Coordinator:
- Add schema field to datastore, add migration path from no-schema.
- Move log stream validation to Load/Save, remove special handlers.
- Remove archival polling. Set up emergency archive poller instead.
- Archive tasking will happen once, either on termination or on expired
  stream polling, and will be recorded in the stream's state.
- Clean up some tests to accommodate the changes.

Archivist:
- ONLY clear archive task when acknowledged by the Coordinator, since it
  will not be recreated.
- Archival tasks will now note the age when incomplete archival is
  permitted.

Collector:
- Add a "don't waste your time" quick path to discard a log bundle.

Committed: https://github.com/luci/luci-go/commit/c3ca643d9c02f0a7684e73e9aeae77206ca88533

[dnj, 2016-04-06 17:21:10]

dnj@chromium.org changed reviewers:
+ estaab@chromium.org, iannucci@chromium.org, vadimsh@chromium.org

[dnj, 2016-04-06 17:21:11]

PTAL

vadimsh@, this is a replacement for the previous "retry failure" CL that you
reviewer. I've incorporated all of that into this, but this goes a lot further
to address archival backlog issues that we've been hitting under load.

[Vadim Sh., 2016-04-07 00:37:30]

I'll review that since I have a bit of time now.

[dnj, 2016-04-07 00:45:35]

On 2016/04/07 00:37:30, Vadim Sh. wrote:
> I'll review that since I have a bit of time now.

Thanks! Previous CL, for the record: https://codereview.chromium.org/1853433002

[Vadim Sh., 2016-04-07 01:21:32]

https://codereview.chromium.org/1863973002/diff/20001/appengine/logdog/coordi...
File appengine/logdog/coordinator/backend/archiveCron.go (right):

https://codereview.chromium.org/1863973002/diff/20001/appengine/logdog/coordi...
appengine/logdog/coordinator/backend/archiveCron.go:100: createdTask, err :=
params.CreateTask(tq.Get(c), ls, ccfg.ArchiveTaskQueue)
nit: you can check ls.State outside the transaction first.

https://codereview.chromium.org/1863973002/diff/20001/appengine/logdog/coordi...
File appengine/logdog/coordinator/endpoints/services/archiveStream.go (right):

https://codereview.chromium.org/1863973002/diff/20001/appengine/logdog/coordi...
appengine/logdog/coordinator/endpoints/services/archiveStream.go:63: var ierr
error
nit: reset ierr to nil at the beginning of a transaction (since it can be
retried)

https://codereview.chromium.org/1863973002/diff/20001/appengine/logdog/coordi...
File appengine/logdog/coordinator/logStream.go (right):

https://codereview.chromium.org/1863973002/diff/20001/appengine/logdog/coordi...
appengine/logdog/coordinator/logStream.go:195: // noDatastoreValidate is a
testing parameter to instruct the LogStream not to
noDSValidate

https://codereview.chromium.org/1863973002/diff/20001/common/api/logdog_coord...
File common/api/logdog_coordinator/logs/v1/state.proto (right):

https://codereview.chromium.org/1863973002/diff/20001/common/api/logdog_coord...
common/api/logdog_coordinator/logs/v1/state.proto:23: int64 terminal_index = 3;
are you sure changing what tag means won't backfire? Both pRPC server and client
must be updated at the same time.

https://codereview.chromium.org/1863973002/diff/20001/common/api/logdog_coord...
File common/api/logdog_coordinator/services/v1/service.proto (right):

https://codereview.chromium.org/1863973002/diff/20001/common/api/logdog_coord...
common/api/logdog_coordinator/services/v1/service.proto:81: // The number of
entries in the log stream.
in LogStreamState it is "index", here it is "number". What is it?

I mean in a list with 3 items, the terminal index is 2, but "number of entries"
is 3.

https://codereview.chromium.org/1863973002/diff/20001/common/api/logdog_coord...
common/api/logdog_coordinator/services/v1/service.proto:96: bool error = 4;
maybe "string error" // if not empty, there was an archival error...

to have some details about the error stored

https://codereview.chromium.org/1863973002/diff/20001/common/api/logdog_coord...
File common/api/logdog_coordinator/services/v1/tasks.proto (right):

https://codereview.chromium.org/1863973002/diff/20001/common/api/logdog_coord...
common/api/logdog_coordinator/services/v1/tasks.proto:22: // (leaseTimestamp)
from its enqueued timestamp (enqueueTimestamp).
dunno, I'd just use global timestamp and don't bother trying to calculate it
from other fields

if clocks are desyncronized, a lot of bad stuff happens anyway (e.g. SSL may
stop to work). GCE machines are configured to use Google's NTP server by
default, and will almost certainly by in sync with each other and with GAE
clock.

https://codereview.chromium.org/1863973002/diff/20001/server/internal/logdog/...
File server/internal/logdog/archivist/archivist.go (right):

https://codereview.chromium.org/1863973002/diff/20001/server/internal/logdog/...
server/internal/logdog/archivist/archivist.go:59: // is truely complete, as
deleting a named task in the task queue will cause it
typo: truly

also you don't use named task (anymore?), so the comment about 10 days is no
longer relevant

[dnj, 2016-04-11 16:54:21]

Description was changed from

==========
LogDog: Update to archival V2.

Remove archive task polling. The standard path to archival will be archive
tasks created when the stream is registered as terminated. These tasks
will stick around until the archival is successful or has been declared
a failure.

Archival completeness is now a property of its task: if the task is
older than a threshold, the archive's completeness requirement will be
dropped.

Archival also tracks fatal errors (e.g., data corruption) and will
error-archive a stream if this happens a certain number of times.

An emergency poller will still run periodically to catch streams that
have not been terminated, either because of data loss or because the
Butler never sent a terminal index (i.e., it crashed).

Coordinator:
- Add schema field to datastore, add migration path from no-schema.
- Move log stream validation to Load/Save, remove special handlers.
- Remove archival polling. Set up emergency archive poller instead.
- Archive tasking will happen once, either on termination or on expired
  stream polling, and will be recorded in the stream's state.
- Clean up some tests to accommodate the changes.

Archivist:
- ONLY clear archive task when acknowledged by the Coordinator, since it
  will not be recreated.
- Archival tasks will now note the age when incomplete archival is
  permitted.

Collector:
- Add a "don't waste your time" quick path to discard a log bundle.

BUG=
==========

to

==========
LogDog: Update to archival V2.

Remove archive task polling. The standard path to archival will be archive
tasks created when the stream is registered as terminated. These tasks
will stick around until the archival is successful or has been declared
a failure.

Archival completeness is now a property of its task: if the task is
older than a threshold, the archive's completeness requirement will be
dropped.

Archival also offers an error path which will archive an empty stream if
there is a fatal (non-transient) archival error. This primarily targets
data errors.

An emergency poller will still run periodically to catch streams that
have not been terminated, either because of data loss or because the
Butler never sent a terminal index (i.e., it crashed).

We also switch from pull queues (alpha REST API) to Pub/Sub for archival
tasking. A unique key is sent with each Pub/Sub task to recreate
transactional task queue operations in Pub/Sub - only the Pub/Sub message
that includes the key transactionally encoded into Datastore will be
used for archival.

Coordinator:
- Add schema field to datastore, add migration path from no-schema.
- Move log stream validation to Load/Save, remove special handlers.
- Remove archival polling. Set up emergency archive poller instead.
- Archive tasking will happen once, either on termination or on expired
  stream polling, and will be recorded in the stream's state.
- Clean up some tests to accommodate the changes.

Archivist:
- ONLY clear archive task when acknowledged by the Coordinator, since it
  will not be recreated.
- Archival tasks will now note the age when incomplete archival is
  permitted.

Collector:
- Add a "don't waste your time" quick path to discard a log bundle.
==========

[dnj, 2016-04-11 17:20:05]

Responded to comments, thanks a lot, vadimsh@!

I have uploaded another large change. It builds on the previous one, though, so
that was NOT wasted time.

This change:
- Uses Pub/Sub for tasking instead of Pull Queues.
- Rewrites archival pipeline to use a staging space.
- Some quality of life cleanup along the way.

Because this is large (and for posterity's sake), I have written up a document
detailing the total set of changes:
https://docs.google.com/document/d/1OPEnkKK6ALjlurEpLUGRplfLFK0AebQEOEkH1K33W...

You can probably review without the doc, but it's there if you want more info.

https://codereview.chromium.org/1863973002/diff/20001/appengine/logdog/coordi...
File appengine/logdog/coordinator/backend/archiveCron.go (right):

https://codereview.chromium.org/1863973002/diff/20001/appengine/logdog/coordi...
appengine/logdog/coordinator/backend/archiveCron.go:100: createdTask, err :=
params.CreateTask(tq.Get(c), ls, ccfg.ArchiveTaskQueue)
On 2016/04/07 01:21:32, Vadim Sh. wrote:
> nit: you can check ls.State outside the transaction first.

Is this worth doing (and incurring another write cost)? Since it's already
satisfied the query, it seems to me that that strongly indicates that this state
will probably be correct.

https://codereview.chromium.org/1863973002/diff/20001/appengine/logdog/coordi...
File appengine/logdog/coordinator/endpoints/services/archiveStream.go (right):

https://codereview.chromium.org/1863973002/diff/20001/appengine/logdog/coordi...
appengine/logdog/coordinator/endpoints/services/archiveStream.go:63: var ierr
error
On 2016/04/07 01:21:32, Vadim Sh. wrote:
> nit: reset ierr to nil at the beginning of a transaction (since it can be
> retried)

Done.

https://codereview.chromium.org/1863973002/diff/20001/appengine/logdog/coordi...
File appengine/logdog/coordinator/logStream.go (right):

https://codereview.chromium.org/1863973002/diff/20001/appengine/logdog/coordi...
appengine/logdog/coordinator/logStream.go:195: // noDatastoreValidate is a
testing parameter to instruct the LogStream not to
On 2016/04/07 01:21:32, Vadim Sh. wrote:
> noDSValidate

Done.

https://codereview.chromium.org/1863973002/diff/20001/common/api/logdog_coord...
File common/api/logdog_coordinator/logs/v1/state.proto (right):

https://codereview.chromium.org/1863973002/diff/20001/common/api/logdog_coord...
common/api/logdog_coordinator/logs/v1/state.proto:23: int64 terminal_index = 3;
On 2016/04/07 01:21:32, Vadim Sh. wrote:
> are you sure changing what tag means won't backfire? Both pRPC server and
client
> must be updated at the same time. 

The only thing that consumes this ATM is JSONPB web UI, so as long as the field
name hasn't changed, we should be good.

https://codereview.chromium.org/1863973002/diff/20001/common/api/logdog_coord...
File common/api/logdog_coordinator/services/v1/service.proto (right):

https://codereview.chromium.org/1863973002/diff/20001/common/api/logdog_coord...
common/api/logdog_coordinator/services/v1/service.proto:81: // The number of
entries in the log stream.
On 2016/04/07 01:21:32, Vadim Sh. wrote:
> in LogStreamState it is "index", here it is "number". What is it?
> 
> I mean in a list with 3 items, the terminal index is 2, but "number of
entries"
> is 3.

Sorry I was considering changing this, then reverted that decision, but I must
have forgotten to update the comment. Will fix.

https://codereview.chromium.org/1863973002/diff/20001/common/api/logdog_coord...
common/api/logdog_coordinator/services/v1/service.proto:96: bool error = 4;
On 2016/04/07 01:21:32, Vadim Sh. wrote:
> maybe "string error" // if not empty, there was an archival error...
> 
> to have some details about the error stored

I don't want to retain the error in datastore, so the only advantage this would
offer is to have the error be easily accessible in GAE logs. I suppose that's
worthwhile, though.

https://codereview.chromium.org/1863973002/diff/20001/common/api/logdog_coord...
File common/api/logdog_coordinator/services/v1/tasks.proto (right):

https://codereview.chromium.org/1863973002/diff/20001/common/api/logdog_coord...
common/api/logdog_coordinator/services/v1/tasks.proto:22: // (leaseTimestamp)
from its enqueued timestamp (enqueueTimestamp).
On 2016/04/07 01:21:32, Vadim Sh. wrote:
> dunno, I'd just use global timestamp and don't bother trying to calculate it
> from other fields
> 
> if clocks are desyncronized, a lot of bad stuff happens anyway (e.g. SSL may
> stop to work). GCE machines are configured to use Google's NTP server by
> default, and will almost certainly by in sync with each other and with GAE
> clock.

Since we have to load the stream state in the Archivist anyway, I'm replacing
this mechanism with just having the Coordinator supply the age as one of the
results of that RPC.

(But yes generically agree)

https://codereview.chromium.org/1863973002/diff/20001/server/internal/logdog/...
File server/internal/logdog/archivist/archivist.go (right):

https://codereview.chromium.org/1863973002/diff/20001/server/internal/logdog/...
server/internal/logdog/archivist/archivist.go:59: // is truely complete, as
deleting a named task in the task queue will cause it
On 2016/04/07 01:21:32, Vadim Sh. wrote:
> typo: truly
> 
> also you don't use named task (anymore?), so the comment about 10 days is no
> longer relevant

Done.

https://codereview.chromium.org/1863973002/diff/40001/appengine/logdog/coordi...
File appengine/logdog/coordinator/archivalPublisher.go (right):

https://codereview.chromium.org/1863973002/diff/40001/appengine/logdog/coordi...
appengine/logdog/coordinator/archivalPublisher.go:19: type ArchivalPublisher
interface {
This exists so we can mock it in tests and assert that various archival jobs
were correctly dispatched. It is also sufficiently decoupled from Pub/Sub that
if we switch tasking mechanisms, it will hopefully be far less intrusive.

https://codereview.chromium.org/1863973002/diff/40001/appengine/logdog/coordi...
File appengine/logdog/coordinator/backend/archiveCron.go (right):

https://codereview.chromium.org/1863973002/diff/40001/appengine/logdog/coordi...
appengine/logdog/coordinator/backend/archiveCron.go:100: if err :=
params.PublishTask(c, ap, &ls); err != nil {
PublishTask is transaction-friendly, as it internally increments a counter to
make each dispatch use a unique key. It will also update "ls"'s state.

https://codereview.chromium.org/1863973002/diff/40001/appengine/logdog/coordi...
appengine/logdog/coordinator/backend/archiveCron.go:148: return
errors.New("failed to archive all candidate streams")
Since this is cron, we're basically just using the UI here to show problems.

https://codereview.chromium.org/1863973002/diff/40001/appengine/logdog/coordi...
File appengine/logdog/coordinator/config/config.go (right):

https://codereview.chromium.org/1863973002/diff/40001/appengine/logdog/coordi...
appengine/logdog/coordinator/config/config.go:62: gcfg := GlobalConfig{}
This was changed b/c previously we were loading (usually from cache) a test blob
and unmarshalling it each time we wanted to see something. Now we cache it in
Services.

I also removed the GlobalConfig -> Config helper function. Now you get a
GlobalConfig, use it to instantiate a Config, and then use that. This
functionality is wrapped by Services.

https://codereview.chromium.org/1863973002/diff/40001/appengine/logdog/coordi...
File appengine/logdog/coordinator/endpoints/logs/get.go (right):

https://codereview.chromium.org/1863973002/diff/40001/appengine/logdog/coordi...
appengine/logdog/coordinator/endpoints/logs/get.go:54: svc := s.GetServices()
All endpoints and handlers begin with this boilerplate now.

https://codereview.chromium.org/1863973002/diff/40001/appengine/logdog/coordi...
File appengine/logdog/coordinator/endpoints/services/archiveStream.go (right):

https://codereview.chromium.org/1863973002/diff/40001/appengine/logdog/coordi...
appengine/logdog/coordinator/endpoints/services/archiveStream.go:33: }.Infof(c,
"Received archival request.")
Makes it much easier to find logs in GAE. I recommend everyone do something like
this.

https://codereview.chromium.org/1863973002/diff/40001/appengine/logdog/coordi...
File appengine/logdog/coordinator/endpoints/services/terminateStream.go (right):

https://codereview.chromium.org/1863973002/diff/40001/appengine/logdog/coordi...
appengine/logdog/coordinator/endpoints/services/terminateStream.go:107: if err
== coordinator.ErrArchiveTasked {
Note that this will never happen, since this error is already precluded by the
state assertion above. However, the API allows for it to happen, so I'm checking
it here.

https://codereview.chromium.org/1863973002/diff/40001/appengine/logdog/coordi...
File appengine/logdog/coordinator/logStream.go (right):

https://codereview.chromium.org/1863973002/diff/40001/appengine/logdog/coordi...
appengine/logdog/coordinator/logStream.go:92: HashID string `gae:"$id"`
Note that this is now a mandatory field. I needed to implement this so
ds.PopulateKey would work here.

Previously the various LogStream creation methods would populate this, or it
would be populated opportunistically when needed.

https://codereview.chromium.org/1863973002/diff/40001/appengine/logdog/coordi...
File appengine/logdog/coordinator/service.go (right):

https://codereview.chromium.org/1863973002/diff/40001/appengine/logdog/coordi...
appengine/logdog/coordinator/service.go:35: type Services interface {
This is a reconstruction of Service that is more versatile, in that it can cache
its results on a per-request basis.

Most of this logic was moved from "/config/".

https://codereview.chromium.org/1863973002/diff/40001/appengine/logdog/coordi...
appengine/logdog/coordinator/service.go:188: func (s *prodServices)
ArchivalPublisher(c context.Context) (ArchivalPublisher, error) {
(Except this, this is new).

https://codereview.chromium.org/1863973002/diff/40001/common/gcloud/gs/gs.go
File common/gcloud/gs/gs.go (right):

https://codereview.chromium.org/1863973002/diff/40001/common/gcloud/gs/gs.go#...
common/gcloud/gs/gs.go:43: NewWriter(p Path) (Writer, error)
This API was originally created before I formally build the Path type. Now that
it exists, it is much nicer than separate bucket/path strings, so I have ported
the API over to it.

https://codereview.chromium.org/1863973002/diff/40001/common/gcloud/gs/gs.go#...
common/gcloud/gs/gs.go:134: func (c *prodClient) Rename(src, dst Path) error {
This is a new API.

https://codereview.chromium.org/1863973002/diff/40001/server/cmd/logdog_colle...
File server/cmd/logdog_collector/main.go (right):

https://codereview.chromium.org/1863973002/diff/40001/server/cmd/logdog_colle...
server/cmd/logdog_collector/main.go:131: switch err {
This was a bugfix ported from Archivist in response to a Pub/Sub API
cancellation bug that I encountered. Namely, when "iterator.Stop" is called, it
doesn't actually send EOF immediately. You have to cancel the context first :(

https://codereview.chromium.org/1863973002/diff/40001/server/internal/logdog/...
File server/internal/logdog/archivist/archivist.go (right):

https://codereview.chromium.org/1863973002/diff/40001/server/internal/logdog/...
server/internal/logdog/archivist/archivist.go:86: func (a *Archivist)
ArchiveTask(c context.Context, task Task) bool {
I re-wrote most of this to use a staging space and work with Pub/Sub tasking. I
think it's a lot cleaner now, though.

https://codereview.chromium.org/1863973002/diff/40001/server/internal/logdog/...
File server/internal/logdog/archivist/storageSource.go (right):

https://codereview.chromium.org/1863973002/diff/40001/server/internal/logdog/...
server/internal/logdog/archivist/storageSource.go:19: type storageSource struct
{
This was pulled out of archivist.go. Nothing really new here.

In a future CL (https://codereview.chromium.org/1874563005) I drop support for
contiguous entries and assert them before archival, making this even simpler.

https://codereview.chromium.org/1863973002/diff/40001/server/internal/logdog/...
File server/internal/logdog/collector/collector.go (right):

https://codereview.chromium.org/1863973002/diff/40001/server/internal/logdog/...
server/internal/logdog/collector/collector.go:172: if len(h.be.Logs) == 0 &&
!h.be.Terminal {
Quick short-circuit here. Nothing really interesting.

https://codereview.chromium.org/1863973002/diff/40001/server/logdog/archive/a...
File server/logdog/archive/archive.go (right):

https://codereview.chromium.org/1863973002/diff/40001/server/logdog/archive/a...
server/logdog/archive/archive.go:151: if errors.Any(err, errors.IsTransient) {
This causes the archival process to forward transient errors (e.g., from Google
Storage Writer, etc.)

https://codereview.chromium.org/1863973002/diff/40001/server/logdog/storage/a...
File server/logdog/storage/archive/storage.go (right):

https://codereview.chromium.org/1863973002/diff/40001/server/logdog/storage/a...
server/logdog/storage/archive/storage.go:63: streamPath gs.Path
This was just updated to use "gs.Path" instead of storing separate bucket/path.

[iannucci, 2016-04-19 00:55:28]

lgtm

https://chromiumcodereview.appspot.com/1863973002/diff/40001/appengine/logdog...
File appengine/logdog/coordinator/archivalPublisher.go (right):

https://chromiumcodereview.appspot.com/1863973002/diff/40001/appengine/logdog...
appengine/logdog/coordinator/archivalPublisher.go:19: type ArchivalPublisher
interface {
On 2016/04/11 17:20:04, dnj wrote:
> This exists so we can mock it in tests and assert that various archival jobs
> were correctly dispatched. It is also sufficiently decoupled from Pub/Sub that
> if we switch tasking mechanisms, it will hopefully be far less intrusive.

Since it's just one function, maybe just make this a type and pass it in? E.g.
type ArchivePublish func(...)

https://chromiumcodereview.appspot.com/1863973002/diff/40001/appengine/logdog...
File appengine/logdog/coordinator/endpoints/services/terminateStream.go (right):

https://chromiumcodereview.appspot.com/1863973002/diff/40001/appengine/logdog...
appengine/logdog/coordinator/endpoints/services/terminateStream.go:107: if err
== coordinator.ErrArchiveTasked {
On 2016/04/11 17:20:04, dnj wrote:
> Note that this will never happen, since this error is already precluded by the
> state assertion above. However, the API allows for it to happen, so I'm
checking
> it here.

Acknowledged.

https://chromiumcodereview.appspot.com/1863973002/diff/40001/common/api/logdo...
File common/api/logdog_coordinator/services/v1/service.proto (right):

https://chromiumcodereview.appspot.com/1863973002/diff/40001/common/api/logdo...
common/api/logdog_coordinator/services/v1/service.proto:104: // This field
serves to indivate that an error occured (being non-empty) and
indicate?

[dnj, 2016-04-20 18:15:10]

Thanks for reviewing!

https://chromiumcodereview.appspot.com/1863973002/diff/40001/appengine/logdog...
File appengine/logdog/coordinator/archivalPublisher.go (right):

https://chromiumcodereview.appspot.com/1863973002/diff/40001/appengine/logdog...
appengine/logdog/coordinator/archivalPublisher.go:19: type ArchivalPublisher
interface {
On 2016/04/19 00:55:28, iannucci wrote:
> On 2016/04/11 17:20:04, dnj wrote:
> > This exists so we can mock it in tests and assert that various archival jobs
> > were correctly dispatched. It is also sufficiently decoupled from Pub/Sub
that
> > if we switch tasking mechanisms, it will hopefully be far less intrusive.
> 
> Since it's just one function, maybe just make this a type and pass it in? E.g.
> type ArchivePublish func(...)

ATM I'm not binding this to Pub/Sub, so some properties (namely the Pub/Sub
topic, see "pubsubArchivalPublisher") are not suitable as function parameters,
so this makes sense to me.

https://chromiumcodereview.appspot.com/1863973002/diff/40001/common/api/logdo...
File common/api/logdog_coordinator/services/v1/service.proto (right):

https://chromiumcodereview.appspot.com/1863973002/diff/40001/common/api/logdo...
common/api/logdog_coordinator/services/v1/service.proto:104: // This field
serves to indivate that an error occured (being non-empty) and
On 2016/04/19 00:55:28, iannucci wrote:
> indicate?

indubitably.

[dnj, 2016-04-20 18:15:58]

The CQ bit was checked by dnj@chromium.org

[dnj, 2016-04-20 18:15:59]

The patchset sent to the CQ was uploaded after l-g-t-m from
iannucci@chromium.org
Link to the patchset: https://chromiumcodereview.appspot.com/1863973002/#ps60001
(title: "Fix proto comment.")

[commit-bot: I haz the power, 2016-04-20 18:16:06]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1863973002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1863973002/60001

[commit-bot: I haz the power, 2016-04-20 18:28:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-20 18:28:19]

Failed to request the patch to try. Please note that binary files are still
unsupported at the moment, this is being worked on.

This could be a Rietveld flake (see http://crbug.com/401833), so if this CL has
no binary files, please re-check commit box.
Thanks for your patience.

Transient error: Can't process patch for file
appengine/logdog/coordinator/logStream_migrate.go.
Diff seems corrupted.

[dnj, 2016-04-20 18:28:48]

The CQ bit was checked by dnj@chromium.org

[commit-bot: I haz the power, 2016-04-20 18:28:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1863973002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1863973002/60001

[commit-bot: I haz the power, 2016-04-20 18:40:39]

Description was changed from

==========
LogDog: Update to archival V2.

Remove archive task polling. The standard path to archival will be archive
tasks created when the stream is registered as terminated. These tasks
will stick around until the archival is successful or has been declared
a failure.

Archival completeness is now a property of its task: if the task is
older than a threshold, the archive's completeness requirement will be
dropped.

Archival also offers an error path which will archive an empty stream if
there is a fatal (non-transient) archival error. This primarily targets
data errors.

An emergency poller will still run periodically to catch streams that
have not been terminated, either because of data loss or because the
Butler never sent a terminal index (i.e., it crashed).

We also switch from pull queues (alpha REST API) to Pub/Sub for archival
tasking. A unique key is sent with each Pub/Sub task to recreate
transactional task queue operations in Pub/Sub - only the Pub/Sub message
that includes the key transactionally encoded into Datastore will be
used for archival.

Coordinator:
- Add schema field to datastore, add migration path from no-schema.
- Move log stream validation to Load/Save, remove special handlers.
- Remove archival polling. Set up emergency archive poller instead.
- Archive tasking will happen once, either on termination or on expired
  stream polling, and will be recorded in the stream's state.
- Clean up some tests to accommodate the changes.

Archivist:
- ONLY clear archive task when acknowledged by the Coordinator, since it
  will not be recreated.
- Archival tasks will now note the age when incomplete archival is
  permitted.

Collector:
- Add a "don't waste your time" quick path to discard a log bundle.
==========

to

==========
LogDog: Update to archival V2.

Remove archive task polling. The standard path to archival will be archive
tasks created when the stream is registered as terminated. These tasks
will stick around until the archival is successful or has been declared
a failure.

Archival completeness is now a property of its task: if the task is
older than a threshold, the archive's completeness requirement will be
dropped.

Archival also offers an error path which will archive an empty stream if
there is a fatal (non-transient) archival error. This primarily targets
data errors.

An emergency poller will still run periodically to catch streams that
have not been terminated, either because of data loss or because the
Butler never sent a terminal index (i.e., it crashed).

We also switch from pull queues (alpha REST API) to Pub/Sub for archival
tasking. A unique key is sent with each Pub/Sub task to recreate
transactional task queue operations in Pub/Sub - only the Pub/Sub message
that includes the key transactionally encoded into Datastore will be
used for archival.

Coordinator:
- Add schema field to datastore, add migration path from no-schema.
- Move log stream validation to Load/Save, remove special handlers.
- Remove archival polling. Set up emergency archive poller instead.
- Archive tasking will happen once, either on termination or on expired
  stream polling, and will be recorded in the stream's state.
- Clean up some tests to accommodate the changes.

Archivist:
- ONLY clear archive task when acknowledged by the Coordinator, since it
  will not be recreated.
- Archival tasks will now note the age when incomplete archival is
  permitted.

Collector:
- Add a "don't waste your time" quick path to discard a log bundle.

Committed:
https://github.com/luci/luci-go/commit/c3ca643d9c02f0a7684e73e9aeae77206ca88533
==========

[commit-bot: I haz the power, 2016-04-20 18:40:41]

Committed patchset #4 (id:60001) as
https://github.com/luci/luci-go/commit/c3ca643d9c02f0a7684e73e9aeae77206ca88533