Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/203#discussion_r67530836
--- Diff:
core/src/main/java/org/apache/brooklyn/core/mgmt/persist/FileBasedObjectStore.java
---
@@ -94,25 +94,19 @@ public File getBaseDir() {
}
public void prepareForMasterUse() {
- if (doneFirstContentiousWrite.get())
- return;
- synchronized (this) {
- if (doneFirstContentiousWrite.get())
- return;
- try {
- if (deferredBackupNeeded) {
- // defer backup and path creation until first write
- // this way if node is standby or auto, the backup is
not created superfluously
+ if (doneFirstContentiousWrite.getAndSet(true)) return;
--- End diff --
This also changes the semantics - I haven't dug deep enough to confirm that
it's right.
I'm assuming that nothing else is synchronizing on this object (from some
other class), but it's very hard to tell - one of the main reasons I hate
`synchronized (this)`.
Previously, it only set `doneFirstContentiousWrite` if the call to
`backupDirByCopying` was successful. Otherwise, it would try again in the next
thread that called `prepareForMasterUse()`.
I think that your behaviour is better @grkvlt .
Thoughts?
@ahgittin - anything to add (I think you wrote this original code)?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
