From 67214202f155c0095d743f27d53be5e6e890d247 Mon Sep 17 00:00:00 2001
From: Karen Etheridge <ether@joyent.com>
Date: Tue, 16 Oct 2018 14:24:52 -0700
Subject: [PATCH 1/4] cascade deletes as needed

---
 lib/Conch/DB/Result/ValidationState.pm            |  6 +++---
 lib/Conch/DB/Result/ValidationStateMember.pm      |  6 +++---
 .../0074-device_report-cascade-delete.sql         | 15 +++++++++++++++
 sql/schema.sql                                    |  4 ++--
 4 files changed, 23 insertions(+), 8 deletions(-)
 create mode 100644 sql/migrations/0074-device_report-cascade-delete.sql

diff --git a/lib/Conch/DB/Result/ValidationState.pm b/lib/Conch/DB/Result/ValidationState.pm
index 49edc0df1..c83ee4a9e 100644
--- a/lib/Conch/DB/Result/ValidationState.pm
+++ b/lib/Conch/DB/Result/ValidationState.pm
@@ -152,7 +152,7 @@ __PACKAGE__->belongs_to(
   "device_report",
   "Conch::DB::Result::DeviceReport",
   { id => "device_report_id" },
-  { is_deferrable => 0, on_delete => "NO ACTION", on_update => "NO ACTION" },
+  { is_deferrable => 0, on_delete => "CASCADE", on_update => "NO ACTION" },
 );
 
 =head2 validation_plan
@@ -200,8 +200,8 @@ __PACKAGE__->many_to_many(
 );
 
 
-# Created by DBIx::Class::Schema::Loader v0.07049 @ 2018-10-10 16:06:16
-# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:gkC6RKtvMTPS5Y1V8IlugA
+# Created by DBIx::Class::Schema::Loader v0.07049 @ 2018-10-16 13:17:28
+# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:igN8sWgHS98xZZsbqBdZNQ
 
 __PACKAGE__->add_columns(
     '+created' => { retrieve_on_insert => 1 },
diff --git a/lib/Conch/DB/Result/ValidationStateMember.pm b/lib/Conch/DB/Result/ValidationStateMember.pm
index 002adbd32..1d830352b 100644
--- a/lib/Conch/DB/Result/ValidationStateMember.pm
+++ b/lib/Conch/DB/Result/ValidationStateMember.pm
@@ -94,12 +94,12 @@ __PACKAGE__->belongs_to(
   "validation_state",
   "Conch::DB::Result::ValidationState",
   { id => "validation_state_id" },
-  { is_deferrable => 0, on_delete => "NO ACTION", on_update => "NO ACTION" },
+  { is_deferrable => 0, on_delete => "CASCADE", on_update => "NO ACTION" },
 );
 
 
-# Created by DBIx::Class::Schema::Loader v0.07049 @ 2018-09-17 14:52:33
-# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:3aEz5aEgtuv96u7AJIl+Lg
+# Created by DBIx::Class::Schema::Loader v0.07049 @ 2018-10-16 13:17:28
+# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:waeVK0lJGoJZbkd640IX7A
 
 
 # You can replace this text with custom code or comments, and it will be preserved on regeneration
diff --git a/sql/migrations/0074-device_report-cascade-delete.sql b/sql/migrations/0074-device_report-cascade-delete.sql
new file mode 100644
index 000000000..0c226a846
--- /dev/null
+++ b/sql/migrations/0074-device_report-cascade-delete.sql
@@ -0,0 +1,15 @@
+SELECT run_migration(74, $$
+
+    -- when device_report is deleted, delete validation_state records that point to it
+    alter table validation_state
+        drop constraint validation_state_device_report_id_fkey,
+        add constraint validation_state_device_report_id_fkey
+            foreign key (device_report_id) references device_report(id) on delete cascade;
+
+    -- when validation_state is deleted, delete validation_state_member records that point to it
+    alter table validation_state_member
+        drop constraint validation_state_member_validation_state_id_fkey,
+        add constraint validation_state_member_validation_state_id_fkey
+            foreign key (validation_state_id) references validation_state(id) on delete cascade;
+
+$$);
diff --git a/sql/schema.sql b/sql/schema.sql
index 49340291b..15fb2067b 100644
--- a/sql/schema.sql
+++ b/sql/schema.sql
@@ -1819,7 +1819,7 @@ ALTER TABLE ONLY public.validation_state
 --
 
 ALTER TABLE ONLY public.validation_state
-    ADD CONSTRAINT validation_state_device_report_id_fkey FOREIGN KEY (device_report_id) REFERENCES public.device_report(id);
+    ADD CONSTRAINT validation_state_device_report_id_fkey FOREIGN KEY (device_report_id) REFERENCES public.device_report(id) ON DELETE CASCADE;
 
 
 --
@@ -1835,7 +1835,7 @@ ALTER TABLE ONLY public.validation_state_member
 --
 
 ALTER TABLE ONLY public.validation_state_member
-    ADD CONSTRAINT validation_state_member_validation_state_id_fkey FOREIGN KEY (validation_state_id) REFERENCES public.validation_state(id);
+    ADD CONSTRAINT validation_state_member_validation_state_id_fkey FOREIGN KEY (validation_state_id) REFERENCES public.validation_state(id) ON DELETE CASCADE;
 
 
 --

From 98a6cdb246ce20a9eb7bd44ac917276ca43ff209 Mon Sep 17 00:00:00 2001
From: Karen Etheridge <ether@joyent.com>
Date: Fri, 8 Feb 2019 17:03:32 -0800
Subject: [PATCH 2/4] document the accidental but convenient recycling of
 validation_result rows

---
 lib/Conch/ValidationSystem.pm | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/Conch/ValidationSystem.pm b/lib/Conch/ValidationSystem.pm
index 43b2b6cf3..c5556d5da 100644
--- a/lib/Conch/ValidationSystem.pm
+++ b/lib/Conch/ValidationSystem.pm
@@ -259,7 +259,9 @@ sub run_validation_plan ($self, %options) {
         validation_plan_id => $validation_plan->id,
         status => $status,
         completed => \'now()',
-        validation_state_members => [ map { +{ validation_result => $_ } } @validation_results ],
+        # provided column data is used to determine if these result(s) already exist in the db,
+        # and they are reused if so, otherwise they are inserted
+        validation_state_members => [ map +{ validation_result => $_ }, @validation_results ],
     });
 }
 

From e31730363899b98d398dbdfa3a59c96a4fe58be3 Mon Sep 17 00:00:00 2001
From: Karen Etheridge <ether@joyent.com>
Date: Tue, 12 Feb 2019 10:08:47 -0800
Subject: [PATCH 3/4] reorder validation_status_enum for nicer queries

---
 lib/Conch/DB/Result/ValidationResult.pm        |  8 ++++----
 lib/Conch/DB/Result/ValidationState.pm         | 10 ++++------
 sql/migrations/0075-validation_status_enum.sql | 16 ++++++++++++++++
 sql/schema.sql                                 |  6 +++---
 4 files changed, 27 insertions(+), 13 deletions(-)
 create mode 100644 sql/migrations/0075-validation_status_enum.sql

diff --git a/lib/Conch/DB/Result/ValidationResult.pm b/lib/Conch/DB/Result/ValidationResult.pm
index e6ded3b4d..8499eb8a6 100644
--- a/lib/Conch/DB/Result/ValidationResult.pm
+++ b/lib/Conch/DB/Result/ValidationResult.pm
@@ -68,7 +68,7 @@ __PACKAGE__->table("validation_result");
 =head2 status
 
   data_type: 'enum'
-  extra: {custom_type_name => "validation_status_enum",list => ["error","pass","fail","processing"]}
+  extra: {custom_type_name => "validation_status_enum",list => ["error","fail","processing","pass"]}
   is_nullable: 0
 
 =head2 category
@@ -118,7 +118,7 @@ __PACKAGE__->add_columns(
     data_type => "enum",
     extra => {
       custom_type_name => "validation_status_enum",
-      list => ["error", "pass", "fail", "processing"],
+      list => ["error", "fail", "processing", "pass"],
     },
     is_nullable => 0,
   },
@@ -226,8 +226,8 @@ __PACKAGE__->many_to_many(
 );
 
 
-# Created by DBIx::Class::Schema::Loader v0.07049 @ 2018-09-17 14:52:33
-# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:9AqtWUD7GrQfppyKgyIcXw
+# Created by DBIx::Class::Schema::Loader v0.07049 @ 2019-02-12 15:14:57
+# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:eM2SDA/xuqdqeJcwvRRj2A
 
 __PACKAGE__->add_columns(
     '+created' => { is_serializable => 0 },
diff --git a/lib/Conch/DB/Result/ValidationState.pm b/lib/Conch/DB/Result/ValidationState.pm
index c83ee4a9e..cd4d18215 100644
--- a/lib/Conch/DB/Result/ValidationState.pm
+++ b/lib/Conch/DB/Result/ValidationState.pm
@@ -58,8 +58,7 @@ __PACKAGE__->table("validation_state");
 =head2 status
 
   data_type: 'enum'
-  default_value: 'processing'
-  extra: {custom_type_name => "validation_status_enum",list => ["error","pass","fail","processing"]}
+  extra: {custom_type_name => "validation_status_enum",list => ["error","fail","processing","pass"]}
   is_nullable: 0
 
 =head2 completed
@@ -98,10 +97,9 @@ __PACKAGE__->add_columns(
   "status",
   {
     data_type => "enum",
-    default_value => "processing",
     extra => {
       custom_type_name => "validation_status_enum",
-      list => ["error", "pass", "fail", "processing"],
+      list => ["error", "fail", "processing", "pass"],
     },
     is_nullable => 0,
   },
@@ -200,8 +198,8 @@ __PACKAGE__->many_to_many(
 );
 
 
-# Created by DBIx::Class::Schema::Loader v0.07049 @ 2018-10-16 13:17:28
-# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:igN8sWgHS98xZZsbqBdZNQ
+# Created by DBIx::Class::Schema::Loader v0.07049 @ 2019-02-12 15:14:57
+# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:62AGs6yNbvUQTQWAr0xhCA
 
 __PACKAGE__->add_columns(
     '+created' => { retrieve_on_insert => 1 },
diff --git a/sql/migrations/0075-validation_status_enum.sql b/sql/migrations/0075-validation_status_enum.sql
new file mode 100644
index 000000000..ab52c3fb9
--- /dev/null
+++ b/sql/migrations/0075-validation_status_enum.sql
@@ -0,0 +1,16 @@
+SELECT run_migration(75, $$
+
+    alter type validation_status_enum rename to _validation_status_enum_old;
+
+    create type validation_status_enum as enum ('error','fail','processing','pass');
+
+    alter table validation_result
+        alter column status type validation_status_enum using status::text::validation_status_enum;
+
+    alter table validation_state
+        alter column status drop default,
+        alter column status type validation_status_enum using status::text::validation_status_enum;
+
+    drop type _validation_status_enum_old;
+
+$$);
diff --git a/sql/schema.sql b/sql/schema.sql
index 15fb2067b..3754f93e4 100644
--- a/sql/schema.sql
+++ b/sql/schema.sql
@@ -76,9 +76,9 @@ ALTER TYPE public.user_workspace_role_enum OWNER TO conch;
 
 CREATE TYPE public.validation_status_enum AS ENUM (
     'error',
-    'pass',
     'fail',
-    'processing'
+    'processing',
+    'pass'
 );
 
 
@@ -691,7 +691,7 @@ CREATE TABLE public.validation_state (
     device_id text NOT NULL,
     validation_plan_id uuid NOT NULL,
     created timestamp with time zone DEFAULT now() NOT NULL,
-    status public.validation_status_enum DEFAULT 'processing'::public.validation_status_enum NOT NULL,
+    status public.validation_status_enum NOT NULL,
     completed timestamp with time zone,
     device_report_id uuid NOT NULL
 );

From edb28c1e311fd34147022b710a70eaedb2a3ab54 Mon Sep 17 00:00:00 2001
From: Karen Etheridge <ether@joyent.com>
Date: Fri, 1 Feb 2019 14:10:47 -0800
Subject: [PATCH 4/4] run all incoming reports through validations, and then
 possibly delete older passing reports

We do not keep a device report if it follows a passing result and also is followed by a passing
result. This means that we consider *previous* reports for deletion, never the most recent one.

- remove duplicate device report detection logic (see #460, #492): we now once again process
  all device reports as they come in.
- reorder validation_status_enum for nicer queries
- create a command-line script to thin out historical reports.

closes #619, #461.
---
 cpanfile                                     |   1 +
 cpanfile.snapshot                            |  21 ++
 lib/Conch/Command/thin_device_reports.pm     | 210 +++++++++++++++++++
 lib/Conch/Controller/DeviceReport.pm         |  90 ++++----
 lib/Conch/DB/Result/DeviceReport.pm          |  33 +--
 lib/Conch/DB/ResultSet.pm                    |   1 +
 lib/Conch/DB/ResultSet/DeviceReport.pm       |  25 +++
 sql/migrations/0076-device_report-retain.sql |   7 +
 sql/schema.sql                               |   5 +-
 t/integration/04_test_datacenter_loaded.t    | 203 ++++++++++++------
 10 files changed, 458 insertions(+), 138 deletions(-)
 create mode 100644 lib/Conch/Command/thin_device_reports.pm
 create mode 100644 sql/migrations/0076-device_report-retain.sql

diff --git a/cpanfile b/cpanfile
index 879cfe247..238085a86 100644
--- a/cpanfile
+++ b/cpanfile
@@ -79,6 +79,7 @@ on 'test' => sub {
     requires 'Test::Warnings';
     requires 'Test::Fatal';
     requires 'Test::Deep';
+    requires 'Test::Deep::JSON';
     requires 'Test::Memory::Cycle';
     requires 'Module::CPANfile';
     requires 'DBIx::Class::EasyFixture', '0.13';    # Moo not Moose
diff --git a/cpanfile.snapshot b/cpanfile.snapshot
index 4297216be..34311e720 100644
--- a/cpanfile.snapshot
+++ b/cpanfile.snapshot
@@ -1583,6 +1583,16 @@ DISTRIBUTIONS
       perl 5.008001
       strict 0
       warnings 0
+  Exporter-Lite-0.08
+    pathname: N/NE/NEILB/Exporter-Lite-0.08.tar.gz
+    provides:
+      Exporter::Lite 0.08
+    requirements:
+      Carp 0
+      ExtUtils::MakeMaker 6.3
+      perl 5.006
+      strict 0
+      warnings 0
   Exporter-Tiny-1.002001
     pathname: T/TO/TOBYINK/Exporter-Tiny-1.002001.tar.gz
     provides:
@@ -3070,6 +3080,17 @@ DISTRIBUTIONS
       List::Util 1.09
       Scalar::Util 1.09
       Test::Builder 0
+  Test-Deep-JSON-0.05
+    pathname: M/MO/MOTEMEN/Test-Deep-JSON-0.05.tar.gz
+    provides:
+      Test::Deep::JSON 0.05
+    requirements:
+      Exporter::Lite 0
+      ExtUtils::MakeMaker 6.59
+      JSON::MaybeXS 0
+      Module::Build::Tiny 0.035
+      Test::Deep 0
+      perl 5.008001
   Test-Differences-0.64
     pathname: D/DC/DCANTRELL/Test-Differences-0.64.tar.gz
     provides:
diff --git a/lib/Conch/Command/thin_device_reports.pm b/lib/Conch/Command/thin_device_reports.pm
new file mode 100644
index 000000000..97d2cd5cd
--- /dev/null
+++ b/lib/Conch/Command/thin_device_reports.pm
@@ -0,0 +1,210 @@
+package Conch::Command::thin_device_reports;
+
+=pod
+
+=head1 NAME
+
+thin_device_reports - remove unwanted device reports
+
+=head1 SYNOPSIS
+
+    bin/conch thin_device_reports [long options...]
+
+        --help  print usage message and exit
+
+=cut
+
+use Mojo::Base 'Mojolicious::Command', -signatures;
+use Getopt::Long::Descriptive;
+use Try::Tiny;
+
+has description => 'remove unwanted device reports';
+
+has usage => sub { shift->extract_usage };  # extracts from SYNOPSIS
+
+has 'dry_run';
+
+sub run ($self, @opts) {
+    # if the user needs to ^C, print the post-processing statistics before exiting.
+    local $SIG{INT} = sub {
+        say "\naborting! We now have this many records:";
+        $self->_print_stats;
+        exit;
+    };
+
+    local @ARGV = @opts;
+    my ($opt, $usage) = describe_options(
+        # the descriptions aren't actually used anymore (mojo uses the synopsis instead)... but
+        # the 'usage' text block can be accessed with $usage->text
+        'thin_device_reports %o',
+        [ 'dry-run|n',      'dry-run (no changes are made)' ],
+        [],
+        [ 'help',           'print usage message and exit', { shortcircuit => 1 } ],
+    );
+
+    $self->dry_run($opt->dry_run);
+
+    say 'at start, we have this many records:';
+    $self->_print_stats;
+
+    # consider each device, oldest devices first, in pages of 100 rows each
+    my $device_rs = ($self->dry_run ? $self->app->db_ro_devices : $self->app->db_devices)
+        ->active
+        ->rows(100)
+        ->page(1)
+        ->order_by('created');
+
+    my ($device_count, $device_reports_deleted, $validation_results_deleted) = (0)x3;
+
+    foreach my $page (1 .. $device_rs->pager->last_page) {
+        $device_rs = $device_rs->page($page);
+        while (my $device = $device_rs->next) {
+            # we process each device's reports in a separate transaction,
+            # so we can abort and resume without redoing everything all over again
+            try {
+                my @deleted = $self->app->schema->txn_do(sub {
+                    $self->_process_device($device);
+                });
+                ++$device_count;
+                $device_reports_deleted += $deleted[0];
+                $validation_results_deleted += $deleted[1];
+            }
+            catch {
+                if ($_ =~ /Rollback failed/) {
+                    local $@ = $_;
+                    die;    # propagate the error
+                }
+                print STDERR "\n", 'aborted processing of device ' . $device->id . ': ', $_, "\n";
+            };
+        }
+    }
+
+    say "\n$device_count devices processed.";
+    say $device_reports_deleted.' device_reports deleted.' if $device_reports_deleted;
+    say $validation_results_deleted.' validation_results deleted.' if $validation_results_deleted;
+
+    say 'at finish, we have this many records:';
+    $self->_print_stats;
+}
+
+sub _print_stats ($self) {
+    say 'device_report:           ', $self->app->db_ro_device_reports->count;
+    say 'validation_state:        ', $self->app->db_ro_validation_states->count;
+    say 'validation_state_member: ', $self->app->db_ro_validation_state_members->count;
+    say 'validation_result:       ', $self->app->db_ro_validation_results->count;
+}
+
+sub _process_device ($self, $device) {
+    my $report_count = 0;
+    print 'device id ', $device->id, ': ';
+
+    # Consider the validation status of all reports, oldest first, in pages of 100 rows each.
+    # Valid reports with no validation results are considered to be a 'pass', i.e. eligible for
+    # deletion.
+    my $device_report_rs = $self->app->db_device_reports
+        ->search({ 'device_report.device_id' => $device->id })
+        ->columns('device_report.id')
+        ->with_report_status
+        ->order_by({ -asc => 'device_report.created' })
+        ->rows(100)
+        ->page(1)
+        ->hri;
+
+    # we only delete reports when we are done, so we can safely iterate through reports
+    # without the pages changing strangely
+    my @delete_report_ids;
+
+    # we push data about reports to the end as we consider each one,
+    # and shift data off at the beginning when we're done
+    # $report_statuses[-1]  current report
+    # $report_statuses[-2]  previous report
+    # $report_statuses[-3]  2 reports ago
+    my @report_statuses;
+
+    foreach my $page (1 .. $device_report_rs->pager->last_page) {
+        $device_report_rs = $device_report_rs->page($page);
+        while (my $device_report = $device_report_rs->next) {
+            ++$report_count;
+            print '.' if $report_count % 100 == 0;
+
+            # capture information about the latest report we just fetched.
+            push @report_statuses, $device_report;
+
+            # we maintain a sliding window of (at least?) 3 reports.
+            # We can consider what to do about the middle report now.
+
+            # prevprev    previous   current     delete previous?
+            # dne         dne        FAIL        0   previous report does not exist
+            # dne         dne        PASS        0   previous report does not exist
+            # dne         FAIL       FAIL        0   keep first
+            # dne         FAIL       PASS        0   keep first
+            # dne         PASS       FAIL        0   keep first
+            # dne         PASS       PASS        0   keep first
+            # FAIL        FAIL       FAIL        0   keep reports that fail
+            # FAIL        FAIL       PASS        0   keep reports that fail
+            # FAIL        PASS       FAIL        0   keep first pass after a failure
+            # FAIL        PASS       PASS        0   keep first pass after a failure
+            # PASS        FAIL       FAIL        0   keep reports that fail
+            # PASS        FAIL       PASS        0   keep reports that fail
+            # PASS        PASS       FAIL        0   last pass before a failure
+            # PASS        PASS       PASS        1
+
+            # we only delete the previous report (index [-2]) iff:
+            # - the current report was a pass
+            # - the previous exists and was a pass
+            # - the previous-previous exists and was a pass
+
+            push @delete_report_ids, $report_statuses[-2]{id}
+                if $report_statuses[-1]{status} eq 'pass'
+                    and $report_statuses[-2] and $report_statuses[-2]{status} eq 'pass'
+                    and $report_statuses[-3] and $report_statuses[-3]{status} eq 'pass';
+
+            # forget about the oldest report if we are watching at least 3.
+            shift @report_statuses if $report_statuses[-3];
+        }
+    }
+
+    print "\n";
+
+    my ($device_reports_deleted, $validation_results_deleted) = (0,0);
+
+    if ($self->dry_run) {
+        say 'Would delete ', scalar(@delete_report_ids), ' reports for device id ', $device->id,
+            ' out of ', $report_count, ' examined.';
+    }
+    else {
+        # delete all reports that we identified for deletion
+        # this may also cause cascade deletes on validation_state, validation_state_member.
+        say 'deleting ', scalar(@delete_report_ids), ' reports for device id ', $device->id,
+            ' out of ', $report_count, ' examined...';
+        $device_reports_deleted = $device
+            ->search_related('device_reports', { id => { -in => \@delete_report_ids } })
+            ->delete;
+
+        # delete all newly-orphaned validation_result rows for this device
+        $validation_results_deleted = $device->search_related('validation_results',
+            { 'validation_state_members.validation_state_id' => undef },
+            { join => 'validation_state_members' },
+        )->delete;
+    }
+
+    print "\n";
+
+    return ($device_reports_deleted, $validation_results_deleted);
+}
+
+1;
+__END__
+
+=pod
+
+=head1 LICENSING
+
+Copyright Joyent, Inc.
+
+This Source Code Form is subject to the terms of the Mozilla Public License,
+v.2.0. If a copy of the MPL was not distributed with this file, You can obtain
+one at http://mozilla.org/MPL/2.0/.
+
+=cut
+# vim: set ts=4 sts=4 sw=4 et :
diff --git a/lib/Conch/Controller/DeviceReport.pm b/lib/Conch/Controller/DeviceReport.pm
index 5a2dbeef4..f6052b3d5 100644
--- a/lib/Conch/Controller/DeviceReport.pm
+++ b/lib/Conch/Controller/DeviceReport.pm
@@ -88,56 +88,19 @@ sub process ($c) {
 
 	my $existing_device = $c->db_devices->active->find($c->stash('device_id'));
 
-	if ($existing_device
-		and $existing_device->latest_report_matches($raw_report)) {
-
-		$existing_device->self_rs->latest_device_report->update({
-			last_received => \'now()',
-			received_count => \'received_count + 1',
-		});
-
-		if ($unserialized_report->{relay}) {
-			$existing_device
-				->search_related('device_relay_connections',
-					{ relay_id => $unserialized_report->{relay}{serial} })
-				->update_or_create({ last_seen => \'NOW()' });
-		} else {
-			$c->log->warn('received report without relay id (device_id '. $existing_device->id.')');
-		}
-
-		# this magically DTRT, without having to inject a ->as_subselect_rs,
-		# because DBIx::Class::ResultSet::_chain_relationship understands how to wrap
-		# joins using order by/limit into a subquery
-		my $validation_state = $existing_device->self_rs->latest_device_report
-			->related_resultset('validation_states')
-			->order_by({ -desc => 'validation_states.created' })
-			->rows(1)
-			->single;
-
-		if (not $validation_state) {
-			# normally we should always find an associated validation_state record, because all
-			# incoming device reports (that get stored) have validations run against them.
-			$c->log->warn('Duplicate device report detected (device_report_id '
-				. $existing_device->self_rs->latest_device_report->get_column('id')->single
-				. ' but could not find an associated validation_state record to return');
-
-			# but we can try harder to find *something* to return, in most cases...
-			$validation_state = $c->db_device_reports
-				->matches_jsonb($raw_report)
-				->related_resultset('validation_states')
-				->order_by({ -desc => 'validation_states.created' })
-				->rows(1)
-				->single;
-
-			return $c->status(400, { error => 'duplicate report; could not find relevant validation_state record to return from matching reports' }) if not $validation_state;
-		}
-
-		$c->log->debug('Duplicate device report detected (device_report_id '
-			. $validation_state->device_report_id
-			. '; returning previous validation_state (id ' . $validation_state->id .')');
-
-		return $c->status(200, $validation_state);
-	}
+    # capture information about the last report before we store the new one
+    # state can be: error, fail, processing, pass, where no validations on a valid report is
+    # considered to be a pass.
+    my ($previous_report_id, $previous_report_status);
+    if ($existing_device) {
+        ($previous_report_id, $previous_report_status) =
+            $existing_device->self_rs->latest_device_report
+                ->columns('device_reports.id')
+                ->with_report_status
+                ->hri
+                ->single
+                ->@{qw(id status)};
+    }
 
 	# Update/create the device and create the device report
 	$c->log->debug("Updating or creating device ".$c->stash('device_id'));
@@ -161,7 +124,10 @@ sub process ($c) {
 	$c->log->debug("Creating device report");
 	my $device_report = $device->create_related('device_reports', {
 		report    => $raw_report,
-		# invalid, created, last_received, received_count all use defaults.
+		# we will always keep this report if the previous report failed, or this is the first
+		# report (in its phase).
+		!$previous_report_status || $previous_report_status ne 'pass' ? ( retain => 1 ) : (),
+		# invalid, created use defaults.
 	});
 	$c->log->info("Created device report ".$device_report->id);
 
@@ -208,6 +174,28 @@ sub process ($c) {
 
 	$device->update( { health => uc( $validation_state->status ), updated => \'NOW()' } );
 
+    # save some state about this report that will help us out next time, when we consider
+    # deleting it...  we always keep all failing reports (we also keep the first report after a
+    # failure)
+    $device_report->update({ retain => 1 })
+        if $validation_state->status ne 'pass' and not $device_report->retain;
+
+    # now delete that previous report, if we can
+    if ($previous_report_id and $previous_report_status eq 'pass') {
+        if ($c->db_device_reports
+            ->search({ id => $previous_report_id, retain => \'is not TRUE' })
+            ->delete > 0)
+        {
+            $c->log->debug('deleted previous device report id '.$previous_report_id);
+            # deleting device_report cascaded to validation_state and validation_state_member;
+            # now clean up orphaned results
+            $device->search_related('validation_results',
+                { 'validation_state_members.validation_state_id' => undef },
+                { join => 'validation_state_members' },
+            )->delete;
+        }
+    }
+
 	$c->status( 200, $validation_state );
 }
 
diff --git a/lib/Conch/DB/Result/DeviceReport.pm b/lib/Conch/DB/Result/DeviceReport.pm
index f64772acf..825ddc37b 100644
--- a/lib/Conch/DB/Result/DeviceReport.pm
+++ b/lib/Conch/DB/Result/DeviceReport.pm
@@ -53,24 +53,16 @@ __PACKAGE__->table("device_report");
   is_nullable: 0
   original: {default_value => \"now()"}
 
-=head2 last_received
-
-  data_type: 'timestamp with time zone'
-  default_value: current_timestamp
-  is_nullable: 0
-  original: {default_value => \"now()"}
-
-=head2 received_count
-
-  data_type: 'integer'
-  default_value: 1
-  is_nullable: 0
-
 =head2 invalid_report
 
   data_type: 'text'
   is_nullable: 1
 
+=head2 retain
+
+  data_type: 'boolean'
+  is_nullable: 1
+
 =cut
 
 __PACKAGE__->add_columns(
@@ -92,17 +84,10 @@ __PACKAGE__->add_columns(
     is_nullable   => 0,
     original      => { default_value => \"now()" },
   },
-  "last_received",
-  {
-    data_type     => "timestamp with time zone",
-    default_value => \"current_timestamp",
-    is_nullable   => 0,
-    original      => { default_value => \"now()" },
-  },
-  "received_count",
-  { data_type => "integer", default_value => 1, is_nullable => 0 },
   "invalid_report",
   { data_type => "text", is_nullable => 1 },
+  "retain",
+  { data_type => "boolean", is_nullable => 1 },
 );
 
 =head1 PRIMARY KEY
@@ -150,8 +135,8 @@ __PACKAGE__->has_many(
 );
 
 
-# Created by DBIx::Class::Schema::Loader v0.07049 @ 2018-11-05 16:57:29
-# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:c9QN/7Wi12rFMh5GqBENNQ
+# Created by DBIx::Class::Schema::Loader v0.07049 @ 2019-02-12 15:36:24
+# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:+QPeM7zTnRVpPfuHsLXs8A
 
 # You can replace this text with custom code or comments, and it will be preserved on regeneration
 1;
diff --git a/lib/Conch/DB/ResultSet.pm b/lib/Conch/DB/ResultSet.pm
index 229e4fed3..ecc2b0298 100644
--- a/lib/Conch/DB/ResultSet.pm
+++ b/lib/Conch/DB/ResultSet.pm
@@ -24,6 +24,7 @@ __PACKAGE__->load_components(
     'Helper::ResultSet::Shortcut::Distinct',    # provides distinct
     '+Conch::DB::ResultsExist',                 # provides exists
     'Helper::ResultSet::Shortcut::Columns',     # provides columns
+    'Helper::ResultSet::Shortcut::Page',        # provides page
 );
 
 1;
diff --git a/lib/Conch/DB/ResultSet/DeviceReport.pm b/lib/Conch/DB/ResultSet/DeviceReport.pm
index bef4ef535..744a39e9b 100644
--- a/lib/Conch/DB/ResultSet/DeviceReport.pm
+++ b/lib/Conch/DB/ResultSet/DeviceReport.pm
@@ -32,6 +32,31 @@ sub matches_jsonb ($self, $jsonb) {
     $self->search(\[ "($me.report - $ignore_fields) = (?::jsonb - $ignore_fields)", $jsonb ]);
 }
 
+=head2 with_report_status
+
+Given a resultset indicating one or more report(s), adds a column to the result indicating
+the cumulative status of all the validation state record(s) associated with it (that is, if all
+pass, then return 'pass', otherwise consider if any were 'error' or 'fail').
+
+Reports with no validation results are considered to be a 'pass'.
+
+=cut
+
+sub with_report_status ($self) {
+    my $me = $self->current_source_alias;
+    $self->search(
+        undef,
+        {
+            '+select' => [ {
+                    '' => \qq{case when $me.invalid_report is not null then 'error' else coalesce(min(validation_states.status),'pass') end},
+                    -as => 'status',
+                } ],
+            join => 'validation_states',
+            group_by => "$me.id",
+        },
+    );
+}
+
 1;
 __END__
 
diff --git a/sql/migrations/0076-device_report-retain.sql b/sql/migrations/0076-device_report-retain.sql
new file mode 100644
index 000000000..f28ef7065
--- /dev/null
+++ b/sql/migrations/0076-device_report-retain.sql
@@ -0,0 +1,7 @@
+SELECT run_migration(76, $$
+
+    alter table device_report drop column received_count;
+    alter table device_report drop column last_received;
+    alter table device_report add column retain boolean;
+
+$$);
diff --git a/sql/schema.sql b/sql/schema.sql
index 3754f93e4..7ea32ce13 100644
--- a/sql/schema.sql
+++ b/sql/schema.sql
@@ -384,9 +384,8 @@ CREATE TABLE public.device_report (
     device_id text NOT NULL,
     report jsonb,
     created timestamp with time zone DEFAULT now() NOT NULL,
-    last_received timestamp with time zone DEFAULT now() NOT NULL,
-    received_count integer DEFAULT 1 NOT NULL,
-    invalid_report text
+    invalid_report text,
+    retain boolean
 );
 
 
diff --git a/t/integration/04_test_datacenter_loaded.t b/t/integration/04_test_datacenter_loaded.t
index a4433dfaf..534e14a05 100644
--- a/t/integration/04_test_datacenter_loaded.t
+++ b/t/integration/04_test_datacenter_loaded.t
@@ -5,6 +5,7 @@ use Test::More;
 use Data::UUID;
 use Path::Tiny;
 use Test::Deep;
+use Test::Deep::JSON;
 use Test::Warnings;
 use Mojo::JSON qw(from_json to_json);
 use Test::Conch;
@@ -83,9 +84,9 @@ subtest 'Workspace Rooms' => sub {
 		->json_is('', []);
 };
 
-subtest 'Register relay' => sub {
-	$t->post_ok(
-		'/relay/deadbeef/register',
+subtest 'Device Report' => sub {
+	# register the relay referenced by the report
+	$t->post_ok('/relay/deadbeef/register',
 		json => {
 			serial   => 'deadbeef',
 			version  => '0.0.1',
@@ -94,80 +95,99 @@ subtest 'Register relay' => sub {
 			alias    => 'test relay'
 		}
 	)->status_is(204);
-};
 
-subtest 'Device Report' => sub {
+    # device reports are submitted thusly:
+    # 0: pass
+    # 1: pass (eventually deleted)
+    # 2: pass
+    # 3: - (invalid json)
+    # 4: - (valid json, but does not pass the schema)
+    # 5: pass
+    # 6: error (empty product_name)
+    # 7: pass
+
 	my $good_report = path('t/integration/resource/passing-device-report.json')->slurp_utf8;
 	$t->post_ok('/device/TEST', { 'Content-Type' => 'application/json' }, $good_report)
 		->status_is(200)
 		->json_schema_is('ValidationState')
-		->json_is( '/status', 'pass' );
-
-	my $validation_state_response = $t->tx->res->json;
-
-	my $validation_state = $t->app->db_validation_states->find($validation_state_response->{id});
-	my $device = $t->app->db_devices->find($validation_state_response->{device_id});
-
-	is($validation_state->device_report->device_id, $device->id,
-		'validation_state links to the device_report_id just uploaded');
-
-	cmp_deeply(
-		$device->latest_report_data,
-		from_json($good_report),
-		'json blob stored in the db matches report on disk',
-	);
+		->json_cmp_deeply(superhashof({
+			device_id => 'TEST',
+			status => 'pass',
+			completed => re(qr/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3,9}Z$/),
+		}));
+
+    my (@device_report_ids, @validation_state_ids);
+    push @device_report_ids, $t->tx->res->json->{device_report_id};
+    push @validation_state_ids, $t->tx->res->json->{id};
+
+    my $device = $t->app->db_devices->find('TEST');
+    cmp_deeply(
+        $device->self_rs->latest_device_report->single,
+        methods(
+            id => $device_report_ids[0],
+            device_id => 'TEST',
+            report => json(from_json($good_report)),
+            invalid_report => undef,
+            retain => bool(1),    # first report is always saved
+        ),
+        'stored the report in raw form',
+    );
 
 	is($device->related_resultset('device_reports')->count, 1, 'one device_report row created');
 	is($device->related_resultset('validation_states')->count, 1, 'one validation_state row created');
+	is($t->app->db_validation_results->count, 1, 'one validation result row created');
 	is($device->related_resultset('device_relay_connections')->count, 1, 'one device_relay_connection row created');
 
-	my $dupe_report = to_json(from_json($good_report));
-	isnt($good_report, $dupe_report, 're-encoded report is not string-identical (whitespace was removed)');
 
-	$t->post_ok('/device/TEST', { 'Content-Type' => 'application/json' }, $dupe_report)
-		->status_is(200)
-		->json_schema_is('ValidationState')
-		->json_is('', $validation_state_response, 'duplicate report detected, older state returned');
+    # submit another passing report...
+    $t->post_ok('/device/TEST', { 'Content-Type' => 'application/json' }, $good_report)
+        ->status_is(200)
+        ->json_schema_is('ValidationState')
+        ->json_cmp_deeply(superhashof({
+            device_id => 'TEST',
+            status => 'pass',
+            completed => re(qr/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3,9}Z$/),
+        }));
 
-	is($device->related_resultset('device_reports')->count, 1, 'still just one device_report row');
-	is($device->related_resultset('validation_states')->count, 1, 'still just one validation_state row');
-	is($device->related_resultset('device_relay_connections')->count, 1, 'still just one device_relay_connection');
+    push @device_report_ids, $t->tx->res->json->{device_report_id};
+    push @validation_state_ids, $t->tx->res->json->{id};
 
-	is(
-		$device->related_resultset('device_reports')->rows(1)->get_column('received_count')->single,
-		2,
-		'received_count is incremented',
-	);
+    is($device->related_resultset('device_reports')->count, 2, 'two device_report rows exist');
+    is($device->related_resultset('validation_states')->count, 2, 'two validation_state rows exist');
+    is($t->app->db_validation_results->count, 1, 'the second validation result is the same as the first');
 
-    my $dupe_report_2 = to_json(+{
-        from_json($good_report)->%*,
-        report_id => 'I am here just to trip you up',
-    });
 
-    $t->post_ok('/device/TEST', { 'Content-Type' => 'application/json' }, $dupe_report_2)
+    # submit another passing report (this makes 3)
+    $t->post_ok('/device/TEST', { 'Content-Type' => 'application/json' }, $good_report)
         ->status_is(200)
         ->json_schema_is('ValidationState')
-        ->json_is('', $validation_state_response, 'duplicate report detected, older state returned');
+        ->json_cmp_deeply(superhashof({
+            device_id => 'TEST',
+            status => 'pass',
+            completed => re(qr/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3,9}Z$/),
+        }));
 
-    is($device->related_resultset('device_reports')->count, 1, 'still just one device_report row');
-    is($device->related_resultset('validation_states')->count, 1, 'still just one validation_state row');
-    is($device->related_resultset('device_relay_connections')->count, 1, 'still just one device_relay_connection');
+    push @device_report_ids, $t->tx->res->json->{device_report_id};
+    push @validation_state_ids, $t->tx->res->json->{id};
 
-    is(
-        $device->related_resultset('device_reports')->rows(1)->get_column('received_count')->single,
-        3,
-        'received_count is incremented',
-    );
+    # now the 2nd of the 3 reports should be deleted.
+    is($device->related_resultset('device_reports')->count, 2, 'still just two device_report rows exist');
+    is($device->related_resultset('validation_states')->count, 2, 'still just two validation_state rows exist');
+    is($t->app->db_validation_results->count, 1, 'still just one validation result row exists');
+
+    ok(!$t->app->db_device_reports->search({ id => $device_report_ids[1] })->exists,
+        'second device_report deleted');
+    ok(!$t->app->db_validation_states->search({ id => $validation_state_ids[1] })->exists,
+        'second validation_state deleted');
 
 
-	my $invalid_json_1 = '{"this": 1s n0t v@l,d ǰsøƞ';
+	my $invalid_json_1 = '{"this": 1s n0t v@l,d ǰsøƞ';	# } for brace matching
 	$t->post_ok('/device/TEST', { 'Content-Type' => 'application/json; charset=utf-8' },
 			Encode::encode('UTF-8', $invalid_json_1))
 		->status_is(400);
 
-	my $corrupt_report = $device->self_rs->latest_device_report->single;
 	cmp_deeply(
-		$corrupt_report,
+		$device->self_rs->latest_device_report->single,
 		methods(
 			device_id => 'TEST',
 			report => undef,
@@ -176,6 +196,13 @@ subtest 'Device Report' => sub {
 		'stored the invalid report in raw form',
 	);
 
+    # the device report was saved, but no validations run.
+    push @device_report_ids, $t->app->db_device_reports->order_by({ -desc => 'created' })->rows(1)->get_column('id')->single;
+
+    is($device->related_resultset('device_reports')->count, 3, 'now three device_report rows exist');
+    is($device->related_resultset('validation_states')->count, 2, 'still just two validation_state rows exist');
+    is($t->app->db_validation_results->count, 1, 'still just one validation result row exists');
+
 	$t->get_ok('/device/TEST')
 		->status_is(200)
 		->json_schema_is('DetailedDevice')
@@ -190,9 +217,8 @@ subtest 'Device Report' => sub {
 			json => { foo => 'this 1s v@l,d ǰsøƞ, but violates the schema' })
 		->status_is(400);
 
-	my $invalid_report = $device->self_rs->latest_device_report->single;
 	cmp_deeply(
-		$invalid_report,
+		$device->self_rs->latest_device_report->single,
 		methods(
 			device_id => 'TEST',
 			invalid_report => $invalid_json_2,
@@ -200,6 +226,13 @@ subtest 'Device Report' => sub {
 		'stored the invalid report in raw form',
 	);
 
+    # the device report was saved, but no validations run.
+    push @device_report_ids, $t->app->db_device_reports->order_by({ -desc => 'created' })->rows(1)->get_column('id')->single;
+
+    is($device->related_resultset('device_reports')->count, 4, 'now four device_report rows exist');
+    is($device->related_resultset('validation_states')->count, 2, 'still just two validation_state rows exist');
+    is($t->app->db_validation_results->count, 1, 'still just one validation result row exists');
+
 	$t->get_ok('/device/TEST')
 		->status_is(200)
 		->json_schema_is('DetailedDevice')
@@ -209,12 +242,49 @@ subtest 'Device Report' => sub {
 		->json_is('/invalid_report' => $invalid_json_2);
 
 
+    # submit another passing report...
+    $t->post_ok('/device/TEST', { 'Content-Type' => 'application/json' }, $good_report)
+        ->status_is(200)
+        ->json_schema_is('ValidationState')
+        ->json_cmp_deeply(superhashof({
+            device_id => 'TEST',
+            status => 'pass',
+            completed => re(qr/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3,9}Z$/),
+        }));
+
+    push @device_report_ids, $t->tx->res->json->{device_report_id};
+    push @validation_state_ids, $t->tx->res->json->{id};
+
+    cmp_deeply(
+        $device->self_rs->latest_device_report->single,
+        methods(
+            id => $device_report_ids[-1],
+            device_id => 'TEST',
+            report => json(from_json($good_report)),
+            invalid_report => undef,
+            retain => bool(1),    # we keep the first report after an error result
+        ),
+        'stored the report in raw form',
+    );
+
+    is($device->related_resultset('device_reports')->count, 5, 'now five device_report rows exist');
+    is($device->related_resultset('validation_states')->count, 3, 'three validation_state rows exist');
+    is($t->app->db_validation_results->count, 1, 'the latest validation result is the same as the first');
+
+
 	my $error_report = path('t/integration/resource/error-device-report.json')->slurp_utf8;
 	$t->post_ok('/device/TEST', { 'Content-Type' => 'application/json' }, $error_report)
 		->status_is(200)
 		->json_schema_is('ValidationState')
 		->json_is('/status', 'error');
 
+    push @device_report_ids, $t->tx->res->json->{device_report_id};
+    push @validation_state_ids, $t->tx->res->json->{id};
+
+    is($device->related_resultset('device_reports')->count, 6, 'now six device_report rows exist');
+    is($device->related_resultset('validation_states')->count, 4, 'now another validation_state row exists');
+    is($t->app->db_validation_results->count, 2, 'now two validation results rows exist');
+
 	$t->get_ok('/device/TEST')
 		->status_is(200)
 		->json_schema_is('DetailedDevice')
@@ -228,13 +298,26 @@ subtest 'Device Report' => sub {
 		->json_schema_is('ValidationState')
 		->json_is('/status', 'pass');
 
-	$t->get_ok('/device/TEST')
-		->status_is(200)
-		->json_schema_is('DetailedDevice')
-		->json_is('/latest_report/product_name' => 'Joyent-G1')
-		->json_is('/invalid_report' => undef)
-		->json_is('/health' => 'PASS')
-		->json_is('/latest_report_is_invalid' => JSON::PP::false);
+    push @device_report_ids, $t->tx->res->json->{device_report_id};
+    push @validation_state_ids, $t->tx->res->json->{id};
+
+    is($device->related_resultset('device_reports')->count, 7, 'now seven device_report rows exist');
+    is($device->related_resultset('validation_states')->count, 5, 'now four validation_state rows exist');
+    is($t->app->db_validation_results->count, 2, 'still just two validation result rows exist');
+
+
+    cmp_deeply(
+        [ $t->app->db_device_reports->order_by('created')->get_column('id')->all ],
+        [ @device_report_ids[0,2,3,4,5,6,7] ],
+        'kept all device reports except the passing report with a pass on both sides',
+    );
+
+    cmp_deeply(
+        [ $t->app->db_validation_states->order_by('created')->get_column('id')->all ],
+        [ @validation_state_ids[0,2,-3,-2,-1] ],
+        'not every device report had an associated validation_state record',
+    );
+
 
     subtest 'relocate a disk' => sub {
         # move one of the device's disks to a different device (and change another field so it