[Box Backup-commit] [boxbackup/boxbackup] 38a450: More debugging for test_cannot_open_multiple_writa...

Mon Jul 9 20:48:47 BST 2018

  Branch: refs/heads/s3_support
  Home:   https://github.com/boxbackup/boxbackup
  Commit: 38a4507120a95f5642661f0f7a20d63feb4a5437
      https://github.com/boxbackup/boxbackup/commit/38a4507120a95f5642661f0f7a20d63feb4a5437
  Author: Chris Wilson <chris+github at qwirx.com>
  Date:   2018-07-09 (Mon, 09 Jul 2018)

  Changed paths:
    M lib/common/FileStream.cpp
    M lib/common/FileStream.h
    M test/backupstore/testbackupstore.cpp
    M test/bbackupd/testbbackupd.cpp

  Log Message:
  -----------
  More debugging for test_cannot_open_multiple_writable_connections(store)

Log opening and closing of files during test teardown. Improve detail and
consistency for locking messages in FileStream, and log at TRACE level during
teardown too.

Fix test broken by increased logging level.

  Commit: ae6a2aac52ee6e30e2654637c01edd3d1b36ab04
      https://github.com/boxbackup/boxbackup/commit/ae6a2aac52ee6e30e2654637c01edd3d1b36ab04
  Author: Chris Wilson <chris+github at qwirx.com>
  Date:   2018-07-09 (Mon, 09 Jul 2018)

  Changed paths:
    M lib/backupstore/BackupStoreRefCountDatabase.cpp
    M lib/win32/emu.cpp

  Log Message:
  -----------
  Hopefully fix issue with renaming over recently-closed files on Windows

This caused random test failures, especially on AppVeyor in test/backupstore
`test_cannot_open_multiple_writable_connections(store)`, which looked like this:

> [00:12:36] 6: [check fix] WARNING: Exception thrown: CommonException(OSFileError) (Failed to rename temporary refcount database file from testfiles/0_0\backup\01234567\refcount.rdb.rfwX to testfiles/0_0\backup\01234567\refcount.rdb.rfw: Access is denied. (5)) at C:\projects\boxbackup\lib\backupstore\BackupStoreRefCountDatabase.cpp:200

This is evil:

> Even though you've called CloseHandle on the file handle, the
> kernel may still have outstanding references that take a few milliseconds to
> close... Windows is notorious for this issue. sqlite handles the problem by
> retrying the delete operation every 100 milliseconds up to a maximum number.

https://stackoverflow.com/questions/1753209/deletefile-fails-on-recently-closed-file

Note that I've only fixed the issue for rename-over, not deletion. The only
race condition involving deletion that I'm aware of was just before
rename-over, which is no longer required now that we use MoveFileEx, so should
be removed anyway.

  Commit: a10c873cd9262c5c33f78678696df948bad44204
      https://github.com/boxbackup/boxbackup/commit/a10c873cd9262c5c33f78678696df948bad44204
  Author: Chris Wilson <chris+github at qwirx.com>
  Date:   2018-07-09 (Mon, 09 Jul 2018)

  Changed paths:
    M infrastructure/m4/boxbackup_tests.m4

  Log Message:
  -----------
  Enable errors on -Winfinite-recursion and -Warray-bounds

  Commit: ce2141fa7007bf295497c2601ff2eb8dd0beacb4
      https://github.com/boxbackup/boxbackup/commit/ce2141fa7007bf295497c2601ff2eb8dd0beacb4
  Author: Chris Wilson <chris+github at qwirx.com>
  Date:   2018-07-09 (Mon, 09 Jul 2018)

  Changed paths:
    M lib/backupclient/ClientTestUtils.cpp
    M lib/backupclient/ClientTestUtils.h
    M lib/backupstore/StoreTestUtils.cpp
    M lib/backupstore/StoreTestUtils.h
    M test/backupstore/testbackupstore.cpp

  Log Message:
  -----------
  Share more test/backupstore code for specialised tests

Move functions and macros for specialised tests (e.g. setup and teardown) from
test/backupstore to lib/backupclient/ClientTestUtils.

Add a config file argument to RaidAndS3TestSpecs constructor.

  Commit: 432da28ffe96d187b6c369c090391d7f6c91efc2
      https://github.com/boxbackup/boxbackup/commit/432da28ffe96d187b6c369c090391d7f6c91efc2
  Author: Chris Wilson <chris+github at qwirx.com>
  Date:   2018-07-09 (Mon, 09 Jul 2018)

  Changed paths:
    M lib/server/Daemon.cpp

  Log Message:
  -----------
  Fix 00f7a546b4: do not unlock PID file after writing

We want to prevent another daemon from starting and locking the PID file

  Commit: 6160c7a497acc195a61b4ae85831877567dc08c1
      https://github.com/boxbackup/boxbackup/commit/6160c7a497acc195a61b4ae85831877567dc08c1
  Author: Chris Wilson <chris+github at qwirx.com>
  Date:   2018-07-09 (Mon, 09 Jul 2018)

  Changed paths:
    M lib/common/CollectInBufferStream.cpp
    M lib/common/CollectInBufferStream.h

  Log Message:
  -----------
  CollectInBufferStream: allow Seek while in write phase

  Commit: 9a862a634393dd65bdbc62b5323c6c5b2b21a108
      https://github.com/boxbackup/boxbackup/commit/9a862a634393dd65bdbc62b5323c6c5b2b21a108
  Author: Chris Wilson <chris+github at qwirx.com>
  Date:   2018-07-09 (Mon, 09 Jul 2018)

  Changed paths:
    M lib/backupstore/BackupFileSystem.cpp
    A lib/common/TeeStream.h

  Log Message:
  -----------
  BackupStoreRefCountDatabase on S3: truncate existing file before overwriting

Otherwise we may download a smaller file from the server, leaving existing
stale data at the end of the old file, and thus fail to match the MD5 checksum
after downloading.

Use a TeeStream to calculate the MD5 checksum while streaming the file, without
needing to read it back again.

  Commit: 423cfdb1b8387689ccc30b916aec6bfd1011d1d4
      https://github.com/boxbackup/boxbackup/commit/423cfdb1b8387689ccc30b916aec6bfd1011d1d4
  Author: Chris Wilson <chris+github at qwirx.com>
  Date:   2018-07-09 (Mon, 09 Jul 2018)

  Changed paths:
    M lib/backupstore/BackupFileSystem.cpp
    M lib/backupstore/BackupFileSystem.h

  Log Message:
  -----------
  BackupStoreCheck: fix failure to remove all spurious files

The old BackupFileSystem::CheckObjectsDir functions did not account for the
difference between the root directory and the first object directory, and thus
failed to remove spurious files from the root directory.

By checking files incrementally we also avoid building up a huge list of all
files on the store, with its consequent memory usage.

  Commit: d6c5d844856bc75f8d15d6d979cb5a2b33274405
      https://github.com/boxbackup/boxbackup/commit/d6c5d844856bc75f8d15d6d979cb5a2b33274405
  Author: Chris Wilson <chris+github at qwirx.com>
  Date:   2018-07-09 (Mon, 09 Jul 2018)

  Changed paths:
    M lib/backupstore/BackupFileSystem.cpp

  Log Message:
  -----------
  S3BackupFileSystem::ReleaseLock: flush refcount DB back to S3 storage

We shouldn't have to destroy the S3BackupFileSystem to flush the refcount DB.
Keeping it cached while not holding a lock on the store is also probably a bad
idea.

  Commit: db8da69424efabc18a581c6402249c9248255d1b
      https://github.com/boxbackup/boxbackup/commit/db8da69424efabc18a581c6402249c9248255d1b
  Author: Chris Wilson <chris+github at qwirx.com>
  Date:   2018-07-09 (Mon, 09 Jul 2018)

  Changed paths:
    M lib/backupstore/BackupStoreCheck2.cpp

  Log Message:
  -----------
  BackupStoreCheck: improve log messages during checks, and code comments

  Commit: 23c3e1d0475d13ff8ac09a74483bc132590154a3
      https://github.com/boxbackup/boxbackup/commit/23c3e1d0475d13ff8ac09a74483bc132590154a3
  Author: Chris Wilson <chris+github at qwirx.com>
  Date:   2018-07-09 (Mon, 09 Jul 2018)

  Changed paths:
    M lib/backupstore/BackupStoreRefCountDatabase.cpp
    M lib/backupstore/BackupStoreRefCountDatabase.h

  Log Message:
  -----------
  BackupStoreRefCountDatabase: improve comments, add an assertion

  Commit: 59eca609e45c02238cb74cd3c9146b3032ce9fe3
      https://github.com/boxbackup/boxbackup/commit/59eca609e45c02238cb74cd3c9146b3032ce9fe3
  Author: Chris Wilson <chris+github at qwirx.com>
  Date:   2018-07-09 (Mon, 09 Jul 2018)

  Changed paths:
    M lib/backupstore/BackupStoreRefCountDatabase.cpp
    M lib/common/FileStream.cpp
    M lib/common/FileStream.h

  Log Message:
  -----------
  BackupStoreRefCountDatabase: truncate on save

Avoids leaving stale data after the last used object ID. It's probably not
important, but it makes the tests happy.

  Commit: 8a8bb8182d5ae70a4c84fc5c71a31bab905838ee
      https://github.com/boxbackup/boxbackup/commit/8a8bb8182d5ae70a4c84fc5c71a31bab905838ee
  Author: Chris Wilson <chris+github at qwirx.com>
  Date:   2018-07-09 (Mon, 09 Jul 2018)

  Changed paths:
    M lib/backupstore/BackupStoreContext.cpp

  Log Message:
  -----------
  BackupStoreContext::AddDirectory: write parent directory immediately

This reduces the risk of losing whole directories and many files because we
don't know where to attach them, because the parent directory was never
uploaded, and the child doesn't record its parent ID.

  Commit: e8d26769247b2f3c1024fc0e34f05da2d1f3c1dc
      https://github.com/boxbackup/boxbackup/commit/e8d26769247b2f3c1024fc0e34f05da2d1f3c1dc
  Author: Chris Wilson <chris+github at qwirx.com>
  Date:   2018-07-09 (Mon, 09 Jul 2018)

  Changed paths:
    M lib/bbackupd/BackupClientContext.cpp

  Log Message:
  -----------
  BackupClientContext::CloseAnyOpenConnection: don't silently ignore errors

Now that we can do significant writeback work during connection cleanup, it's
not appropriate to silently ignore and swallow errors. Really we should retry
this work, but I'm not yet sure what the best retry policy would be.

  Commit: 79cc48cd2894acbd8c8a29ffea1bd20a87f470d8
      https://github.com/boxbackup/boxbackup/commit/79cc48cd2894acbd8c8a29ffea1bd20a87f470d8
  Author: Chris Wilson <chris+github at qwirx.com>
  Date:   2018-07-09 (Mon, 09 Jul 2018)

  Changed paths:
    M lib/bbackupd/BackupClientDirectoryRecord.cpp

  Log Message:
  -----------
  BackupClientDirectoryRecord::UpdateItems: retry on EINTR

Receiving an EINTR usually means that the process received a signal. Depending
on the signal, that might mean that we need to stop, or simply that a timer
expired and we need to send a heartbeat at the next opportunity.

If we haven't been asked to stop, retry the same upload an arbitrary number of
times.  If the keepalive time is too short, we will still fail after 10 times,
but at least now we will log a more helpful error message.

Compare: https://github.com/boxbackup/boxbackup/compare/8b9eb4a10593...79cc48cd2894
      **NOTE:** This service been marked for deprecation: https://developer.github.com/changes/2018-04-25-github-services-deprecation/

      Functionality will be removed from GitHub.com on January 31st, 2019.