https://dev.arvados.org/https://dev.arvados.org/favicon.ico?15576888422017-02-06T19:25:25ZArvadosArvados - Idea #11065: [API] Delete rows from logs table when they exceed a configured thresholdhttps://dev.arvados.org/issues/11065?journal_id=480802017-02-06T19:25:25ZTom Cleggtom@curii.com
<ul><li><strong>Subject</strong> changed from <i>[API] Delete rows from logs table when older than configured threshold</i> to <i>[API] Delete rows from logs table when they exceed a configured threshold</i></li><li><strong>Description</strong> updated (<a title="View differences" href="/journals/48080/diff?detail_id=46262">diff</a>)</li><li><strong>Category</strong> set to <i>API</i></li></ul> Arvados - Idea #11065: [API] Delete rows from logs table when they exceed a configured thresholdhttps://dev.arvados.org/issues/11065?journal_id=480822017-02-06T19:41:30ZTom Cleggtom@curii.com
<ul></ul>TBD:
<ul>
<li>What should the default be? 2w?</li>
<li>Any special considerations for existing sites that <em>want</em> to keep logs forever, and don't check the docs before upgrading?</li>
<li>Any special considerations for the very first "delete old logs" query, which will probably be quite slow on existing sites?</li>
<li><del>Worth using a shorter interval (like threshold/100) or backgrounding the query, to avoid producing inexplicably slow API calls when they happen to land on a garbage-collection threshold?</del> (description updated to include this)</li>
</ul> Arvados - Idea #11065: [API] Delete rows from logs table when they exceed a configured thresholdhttps://dev.arvados.org/issues/11065?journal_id=480992017-02-07T20:59:38ZTom Cleggtom@curii.com
<ul><li><strong>Description</strong> updated (<a title="View differences" href="/journals/48099/diff?detail_id=46281">diff</a>)</li></ul> Arvados - Idea #11065: [API] Delete rows from logs table when they exceed a configured thresholdhttps://dev.arvados.org/issues/11065?journal_id=484192017-02-15T20:43:42ZTom Morristfmorris@veritasgenetics.com
<ul><li><strong>Target version</strong> set to <i>Arvados Future Sprints</i></li></ul> Arvados - Idea #11065: [API] Delete rows from logs table when they exceed a configured thresholdhttps://dev.arvados.org/issues/11065?journal_id=494422017-03-14T18:56:40ZTom Morristfmorris@veritasgenetics.com
<ul><li><strong>Target version</strong> changed from <i>Arvados Future Sprints</i> to <i>2017-04-12 sprint</i></li><li><strong>Story points</strong> changed from <i>0.5</i> to <i>2.0</i></li></ul> Arvados - Idea #11065: [API] Delete rows from logs table when they exceed a configured thresholdhttps://dev.arvados.org/issues/11065?journal_id=494502017-03-14T19:54:49ZTom Cleggtom@curii.com
<ul><li><strong>Description</strong> updated (<a title="View differences" href="/journals/49450/diff?detail_id=47651">diff</a>)</li></ul> Arvados - Idea #11065: [API] Delete rows from logs table when they exceed a configured thresholdhttps://dev.arvados.org/issues/11065?journal_id=496032017-03-15T20:00:19ZTom Morristfmorris@veritasgenetics.com
<ul><li><strong>Tracker</strong> changed from <i>Bug</i> to <i>Idea</i></li></ul> Arvados - Idea #11065: [API] Delete rows from logs table when they exceed a configured thresholdhttps://dev.arvados.org/issues/11065?journal_id=496052017-03-15T20:02:17ZTom Morristfmorris@veritasgenetics.com
<ul><li><strong>Target version</strong> changed from <i>2017-04-12 sprint</i> to <i>2017-03-29 sprint</i></li></ul> Arvados - Idea #11065: [API] Delete rows from logs table when they exceed a configured thresholdhttps://dev.arvados.org/issues/11065?journal_id=496072017-03-15T20:02:38ZTom Cleggtom@curii.com
<ul><li><strong>Assigned To</strong> set to <i>Tom Clegg</i></li></ul> Arvados - Idea #11065: [API] Delete rows from logs table when they exceed a configured thresholdhttps://dev.arvados.org/issues/11065?journal_id=497962017-03-21T08:00:14ZTom Cleggtom@curii.com
<ul><li><strong>Status</strong> changed from <i>New</i> to <i>In Progress</i></li></ul> Arvados - Idea #11065: [API] Delete rows from logs table when they exceed a configured thresholdhttps://dev.arvados.org/issues/11065?journal_id=498052017-03-21T15:40:10ZTom Cleggtom@curii.com
<ul></ul><p>11065-rotate-logs @ <a class="changeset" title="11065: Delete old audit logs." href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/37e995fb7423b2b353c599e2a1b00bda7c29ee6f">37e995fb7423b2b353c599e2a1b00bda7c29ee6f</a></p> Arvados - Idea #11065: [API] Delete rows from logs table when they exceed a configured thresholdhttps://dev.arvados.org/issues/11065?journal_id=499002017-03-23T15:38:21ZRadhika Chippadaradhika@curoverse.com
<ul></ul><ul>
<li>“Time to keep audit logs (a row in the log table added … “ => “Time to keep audit logs (a row in the log table <strong>is</strong> added …”</li>
</ul>
<ul>
<li>Since we are using the default value from “common” in “test” config also, do we really need this at line 483: max_audit_log_delete_batch: 0</li>
</ul>
<ul>
<li>I couldn’t find where we are doing Rails.cache.write for ‘AuditLogs' and so I am not clear how this will work: Rails.cache.fetch('AuditLogs', expires_in: exp)</li>
</ul>
<ul>
<li>Do we need File.owned?(Rails.root.join('tmp')) here? Can it be sufficient if one has write permission and can create the lock file here instead?</li>
</ul>
<ul>
<li>This test name can probably clarified as “retain old audit logs with default setting of max_audit_log_delete_batch=0”</li>
</ul>
<ul>
<li>Test ‘retain old audit logs with default settings’ and 'retain old audit logs with batch=0' are basically doing the same, once we clarify the name of the first test …</li>
</ul>
<ul>
<li>tidy_in_background actually returns if max_audit_log_age configured as zero, but the test 'delete old audit logs in multiple batches' ignores such a restriction by passing it and indeed uses zero, which is confusing. Suggest using a value such as 0.1 instead.</li>
</ul>
<ul>
<li>The test name delete old audit logs with production settings' is a bit misleading because max_age < 5min is not production-like per the max age config parameter documentation and you might want to use 5min instead in this test.</li>
</ul> Arvados - Idea #11065: [API] Delete rows from logs table when they exceed a configured thresholdhttps://dev.arvados.org/issues/11065?journal_id=499322017-03-23T19:19:36ZTom Cleggtom@curii.com
<ul></ul><p>Radhika Chippada wrote:</p>
<blockquote>
<ul>
<li>“Time to keep audit logs (a row in the log table added … “ => “Time to keep audit logs (a row in the log table <strong>is</strong> added …”</li>
</ul>
</blockquote>
<p>Reworded.</p>
<blockquote>
<ul>
<li>Since we are using the default value from “common” in “test” config also, do we really need this at line 483: max_audit_log_delete_batch: 0</li>
</ul>
</blockquote>
<p>I guess not. I had added to ensure the test suite doesn't auto-delete even if we start enabling it by default. But maybe it's less confusing if we wait and only do that if/when we get there.</p>
<blockquote>
<ul>
<li>I couldn’t find where we are doing Rails.cache.write for ‘AuditLogs' and so I am not clear how this will work: Rails.cache.fetch('AuditLogs', expires_in: exp)</li>
</ul>
</blockquote>
<p>The block form of fetch() runs the block and updates the cache with the returned value if the cached value is expired/nonexistent.</p>
<blockquote>
<ul>
<li>Do we need File.owned?(Rails.root.join('tmp')) here? Can it be sufficient if one has write permission and can create the lock file here instead?</li>
</ul>
</blockquote>
<p>This addresses the situation where a rake task runs as root. root <em>can</em> write files to the cache directory, but if it does, the www-data won't be able to update/delete them later.</p>
<blockquote>
<ul>
<li>This test name can probably clarified as “retain old audit logs with default setting of max_audit_log_delete_batch=0”</li>
</ul>
<ul>
<li>Test ‘retain old audit logs with default settings’ and 'retain old audit logs with batch=0' are basically doing the same, once we clarify the name of the first test …</li>
</ul>
</blockquote>
<p>Added comments to clarify purpose of separate tests.</p>
<blockquote>
<ul>
<li>tidy_in_background actually returns if max_audit_log_age configured as zero, but the test 'delete old audit logs in multiple batches' ignores such a restriction by passing it and indeed uses zero, which is confusing. Suggest using a value such as 0.1 instead.</li>
</ul>
</blockquote>
<p>Updated.</p>
<blockquote>
<ul>
<li>The test name delete old audit logs with production settings' is a bit misleading because max_age < 5min is not production-like per the max age config parameter documentation and you might want to use 5min instead in this test.</li>
</ul>
</blockquote>
<p>I just didn't think it was worth changing the test fixtures over it... added a comment to explain the anomaly.</p>
<p>11065-rotate-logs @ <a class="changeset" title="11065: Update comments." href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/6722b420effcab24dffd9b47fa277f8830bd4cca">6722b420effcab24dffd9b47fa277f8830bd4cca</a></p> Arvados - Idea #11065: [API] Delete rows from logs table when they exceed a configured thresholdhttps://dev.arvados.org/issues/11065?journal_id=499382017-03-23T19:51:07ZRadhika Chippadaradhika@curoverse.com
<ul></ul><p>LGTM</p> Arvados - Idea #11065: [API] Delete rows from logs table when they exceed a configured thresholdhttps://dev.arvados.org/issues/11065?journal_id=499402017-03-23T20:05:05ZTom Cleggtom@curii.com
<ul><li><strong>Status</strong> changed from <i>In Progress</i> to <i>Resolved</i></li><li><strong>% Done</strong> changed from <i>0</i> to <i>100</i></li></ul><p>Applied in changeset arvados|commit:4600343d1bff7ac4f7b9f08486541444c31af8b6.</p> Arvados - Idea #11065: [API] Delete rows from logs table when they exceed a configured thresholdhttps://dev.arvados.org/issues/11065?journal_id=733732019-04-17T20:42:29ZTom Morristfmorris@veritasgenetics.com
<ul><li><strong>Related to</strong> <i><a class="issue tracker-1 status-3 priority-4 priority-default closed parent" href="/issues/11509">Bug #11509</a>: [Keep-web] Support CORS requests with Range headers</i> added</li></ul>