[Box Backup-commit] [boxbackup/boxbackup] 228b41: Refactor generated protocol classes

GitHub noreply at github.com
Fri Jul 13 22:23:09 BST 2018


  Branch: refs/heads/s3_support
  Home:   https://github.com/boxbackup/boxbackup
  Commit: 228b41d6a4fdfc238f14431e540df225407ec7be
      https://github.com/boxbackup/boxbackup/commit/228b41d6a4fdfc238f14431e540df225407ec7be
  Author: Chris Wilson <chris+github at qwirx.com>
  Date:   2018-07-13 (Fri, 13 Jul 2018)

  Changed paths:
    M lib/server/makeprotocol.pl.in

  Log Message:
  -----------
  Refactor generated protocol classes

Simplify hierarchy slightly, document it in comments, and fix the bug where
mLastError* were shadowed in Wrapper classes, causing reply error checking to
fail.


  Commit: 57d9696df314f28db37731e5dde74d769c6ab786
      https://github.com/boxbackup/boxbackup/commit/57d9696df314f28db37731e5dde74d769c6ab786
  Author: Chris Wilson <chris+github at qwirx.com>
  Date:   2018-07-13 (Fri, 13 Jul 2018)

  Changed paths:
    M lib/bbackupd/BackupClientContext.h
    M test/bbackupd/testbbackupd.cpp

  Log Message:
  -----------
  Fix null pointer dereference in test/bbackupd

This was due to BackupDaemon calling SetNiceMode when our MockClientContext had
not initialised mapConnection, so it was NULL. Fixed by overriding SetNiceMode.
Added comments to reduce risk of recurrence.


  Commit: b4205ce3bfa47dc7a365cb7c070f1c1cbfc73b71
      https://github.com/boxbackup/boxbackup/commit/b4205ce3bfa47dc7a365cb7c070f1c1cbfc73b71
  Author: Chris Wilson <chris+github at qwirx.com>
  Date:   2018-07-13 (Fri, 13 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: 04f3eacbf545f3fc11f8e872782954f7513a1ac9
      https://github.com/boxbackup/boxbackup/commit/04f3eacbf545f3fc11f8e872782954f7513a1ac9
  Author: Chris Wilson <chris+github at qwirx.com>
  Date:   2018-07-13 (Fri, 13 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: 02ff3152a6f09f8332efdca168e4e719edb32d1a
      https://github.com/boxbackup/boxbackup/commit/02ff3152a6f09f8332efdca168e4e719edb32d1a
  Author: Chris Wilson <chris+github at qwirx.com>
  Date:   2018-07-13 (Fri, 13 Jul 2018)

  Changed paths:
    M infrastructure/m4/boxbackup_tests.m4

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


  Commit: 69ef5c73d775d30589fa5ea19dd4c6e497bc0813
      https://github.com/boxbackup/boxbackup/commit/69ef5c73d775d30589fa5ea19dd4c6e497bc0813
  Author: Chris Wilson <chris+github at qwirx.com>
  Date:   2018-07-13 (Fri, 13 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: 03b35a9517af7f52ea4c2d927781e03cc793bb3f
      https://github.com/boxbackup/boxbackup/commit/03b35a9517af7f52ea4c2d927781e03cc793bb3f
  Author: Chris Wilson <chris+github at qwirx.com>
  Date:   2018-07-13 (Fri, 13 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: c8a8abb541b04dc845fb4a106b1456f5fed3cd32
      https://github.com/boxbackup/boxbackup/commit/c8a8abb541b04dc845fb4a106b1456f5fed3cd32
  Author: Chris Wilson <chris+github at qwirx.com>
  Date:   2018-07-13 (Fri, 13 Jul 2018)

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

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


  Commit: 555040f677e69dca673f2f7d1d6194162f2cb1ec
      https://github.com/boxbackup/boxbackup/commit/555040f677e69dca673f2f7d1d6194162f2cb1ec
  Author: Chris Wilson <chris+github at qwirx.com>
  Date:   2018-07-13 (Fri, 13 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: 9791990ad473a2e2b2d2e62f676b6e64600e285d
      https://github.com/boxbackup/boxbackup/commit/9791990ad473a2e2b2d2e62f676b6e64600e285d
  Author: Chris Wilson <chris+github at qwirx.com>
  Date:   2018-07-13 (Fri, 13 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: 3f06a9dcdea9f183b13bb49ec6eb718d47028250
      https://github.com/boxbackup/boxbackup/commit/3f06a9dcdea9f183b13bb49ec6eb718d47028250
  Author: Chris Wilson <chris+github at qwirx.com>
  Date:   2018-07-13 (Fri, 13 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: c98ab37d61beb85792dba95fcbc00f3e15550979
      https://github.com/boxbackup/boxbackup/commit/c98ab37d61beb85792dba95fcbc00f3e15550979
  Author: Chris Wilson <chris+github at qwirx.com>
  Date:   2018-07-13 (Fri, 13 Jul 2018)

  Changed paths:
    M lib/backupstore/BackupStoreCheck2.cpp

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


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

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

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


  Commit: d93a51ba091f1c5d7afafeccdc4caa1dc5970263
      https://github.com/boxbackup/boxbackup/commit/d93a51ba091f1c5d7afafeccdc4caa1dc5970263
  Author: Chris Wilson <chris+github at qwirx.com>
  Date:   2018-07-13 (Fri, 13 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: d84fe6e96b1206186e5dba2ced300c2eb9581ab6
      https://github.com/boxbackup/boxbackup/commit/d84fe6e96b1206186e5dba2ced300c2eb9581ab6
  Author: Chris Wilson <chris+github at qwirx.com>
  Date:   2018-07-13 (Fri, 13 Jul 2018)

  Changed paths:
    M lib/backupstore/BackupStoreContext.cpp
    M test/backupstore/testbackupstore.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: 15dff97a5297f7375d8fe67bf8503060da8ac132
      https://github.com/boxbackup/boxbackup/commit/15dff97a5297f7375d8fe67bf8503060da8ac132
  Author: Chris Wilson <chris+github at qwirx.com>
  Date:   2018-07-13 (Fri, 13 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: 65e7f908750c91714883687a44ac623a9971eb2b
      https://github.com/boxbackup/boxbackup/commit/65e7f908750c91714883687a44ac623a9971eb2b
  Author: Chris Wilson <chris+github at qwirx.com>
  Date:   2018-07-13 (Fri, 13 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/79cc48cd2894...65e7f908750c
      **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.


More information about the Boxbackup-commit mailing list