> On Feb. 9, 2016, 9:42 p.m., Barry Oglesby wrote: > > My test works with these changes. Two things I wanted to verify: > > > > 1. Do we need to worry about backward compatibility? > > > > 2. Invoking BackupManager finishBackup with abort=true still causes the > > cleanup method to be invoked (in the finally block). Since the abort is > > sent to all members regardless of their state, is it ok to cleanup a > > BackupManager that could still be in the middle of processing a backup?
1. I don't think so. We don't need to worry about rolling upgrades from M1 to M2. Nor we do need to worry about P2P compatibility with old gemfire versions. This message is only sent to peers, so I think we are ok there. 2. That's a good point - we might end up aborting a backup that was started by another member. Let see what I can do about that. - Dan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43346/#review118470 ----------------------------------------------------------- On Feb. 9, 2016, 12:33 a.m., Dan Smith wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/43346/ > ----------------------------------------------------------- > > (Updated Feb. 9, 2016, 12:33 a.m.) > > > Review request for geode and Barry Oglesby. > > > Repository: geode > > > Description > ------- > > If the prepare phase of a backup fails, it can leave the backup in a > prepared state on several nodes. The FinishBackupMessage should always > be sent to cleanup the backup. Adding an abort flag to > FinishBackupMessage and always sending that message regardless of > failures during the prepare. > > > Diffs > ----- > > > gemfire-core/src/main/java/com/gemstone/gemfire/admin/internal/AdminDistributedSystemImpl.java > 5709fce0ad921189ca4d6f5e59c84755547c0677 > > gemfire-core/src/main/java/com/gemstone/gemfire/admin/internal/FinishBackupRequest.java > 38335871ef1117fd490cf3e23eceae762fc2ccd7 > > gemfire-core/src/main/java/com/gemstone/gemfire/internal/admin/remote/AdminResponse.java > 14a201c6cfcbee65fe68e0c4c3f1ce22fce6a6d9 > > gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/GemFireCacheImpl.java > 256e2938ced662e23a965abb5b2d490ef8b9e4fe > > gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/persistence/BackupManager.java > 1a5c7655e76e2c4d4ae296fc2db741edfa196e3e > > gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/beans/DistributedSystemBridge.java > 5034f60cdb738cd927b3f58513c38231374e4e52 > > gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/beans/MemberMBeanBridge.java > dc54c6d6d4aedadb4455cc16cdba40b77f78e88f > > gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/BackupDUnitTest.java > 6b0493ee5fa3927cd06d311b71cf9d366b9998ba > > gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/BackupJUnitTest.java > 616e4d5d1f1b8d8c32b955c8b3be0470102a2ccb > > gemfire-core/src/test/java/com/gemstone/gemfire/management/internal/beans/DistributedSystemBridgeJUnitTest.java > PRE-CREATION > gemfire-core/src/test/java/com/gemstone/gemfire/test/fake/Fakes.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/43346/diff/ > > > Testing > ------- > > > Thanks, > > Dan Smith > >
