aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Maxim Khutornenko" <ma...@apache.org>
Subject Re: Review Request 31171: Saving backups asynchronously.
Date Fri, 20 Feb 2015 22:47:22 GMT


> On Feb. 20, 2015, 5:56 p.m., Joshua Cohen wrote:
> > Is it worth adding a test to StorageBackupTest asserting that we write the backup
asynchronously (i.e. some expectation on the mock executor service)?
> 
> Bill Farner wrote:
>     This is not a bad idea.  I wouldn't venture so far to actually use multiple threads,
but you could have one test case where the executor swallows the task to prove that the backup
is not writen.

This is already tested by relying on a FakeScheduledExecutor mocking the `execute()`. If it
was not mocked tests would fail with:
```
java.lang.AssertionError: 
  Unexpected method call ScheduledExecutorService.execute(org.apache.aurora.scheduler.storage.backup.StorageBackup$StorageBackupImpl$1@2c7614d6):
	at org.easymock.internal.MockInvocationHandler.invoke(MockInvocationHandler.java:44)
	at org.easymock.internal.ObjectMethodsFilter.invoke(ObjectMethodsFilter.java:94)
	at com.sun.proxy.$Proxy7.execute(Unknown Source)
	at org.apache.aurora.scheduler.storage.backup.StorageBackup$StorageBackupImpl.createSnapshot(StorageBackup.java:141)
	...
```


> On Feb. 20, 2015, 5:56 p.m., Joshua Cohen wrote:
> > src/test/java/org/apache/aurora/scheduler/storage/backup/StorageBackupTest.java,
line 75
> > <https://reviews.apache.org/r/31171/diff/2/?file=869997#file869997line75>
> >
> >     I think this assignment obviates the need to assign to a new FakeClock above?

Good catch. Done.


- Maxim


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31171/#review73291
-----------------------------------------------------------


On Feb. 19, 2015, 11:58 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31171/
> -----------------------------------------------------------
> 
> (Updated Feb. 19, 2015, 11:58 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Bill Farner.
> 
> 
> Bugs: AURORA-1108
>     https://issues.apache.org/jira/browse/AURORA-1108
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Wrapped backup file handling into Runnable to handle asynchronously. 
> 
> Refactoring somehow triggered a findbugs warning that I had to address as well:
> "Exceptional return value of java.io.File.delete() ignored in org.apache.aurora.scheduler.storage.backup.StorageBackup$StorageBackupImpl$1.run()"
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/storage/backup/BackupModule.java 71feb5779df5738a92e587f9f66f915961f52d1d

>   src/main/java/org/apache/aurora/scheduler/storage/backup/StorageBackup.java a20378a01575c399c23c86aa784424fc6909c34e

>   src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java ea33037d86f30f0787136f34dad34b88eceb0a4d

>   src/test/java/org/apache/aurora/scheduler/storage/backup/RecoveryTest.java 7602d112d29454608a97ec81de14b6bf0c45df68

>   src/test/java/org/apache/aurora/scheduler/storage/backup/StorageBackupTest.java 15fc4404fa2ace4391e4ddc7153c848bc91d43df

> 
> Diff: https://reviews.apache.org/r/31171/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message