You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by sjcorbett <gi...@git.apache.org> on 2014/10/16 00:12:12 UTC
[GitHub] incubator-brooklyn pull request: Copy-state command fixes
GitHub user sjcorbett opened a pull request:
https://github.com/apache/incubator-brooklyn/pull/248
Copy-state command fixes
* No exception
* Copies nodes directory
* Should fix BrooklynLauncherRebindTestToFiles integration test
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/sjcorbett/incubator-brooklyn fixes
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-brooklyn/pull/248.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #248
----
commit 8a703f1dcf188969df081f63599d5715d14b427c
Author: Sam Corbett <sa...@cloudsoftcorp.com>
Date: 2014-10-15T17:40:10Z
BROOKLYN-79: enable write access to persistence for BrooklynLauncher
And no need for OSGi when copying state
commit 5d245289b48c633168d88b4884c70979648d8aa1
Author: Sam Corbett <sa...@cloudsoftcorp.com>
Date: 2014-10-15T22:11:08Z
BROOKLYN-78: copy-state command copies nodes directory
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Copy-state command fixes
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/248#discussion_r18968981
--- Diff: usage/launcher/src/main/java/brooklyn/launcher/BrooklynLauncher.java ---
@@ -449,35 +453,65 @@ public BrooklynLauncher haHeartbeatPeriod(Duration val) {
return this;
}
- public BrooklynMementoRawData retrieveState() {
- initManagementContext();
+ public void copyPersistedState(String destinationDir, String destinationLocation) {
+ copyPersistedState(destinationDir, destinationLocation, null);
+ }
+ public void copyPersistedState(String destinationDir, String destinationLocation, CompoundTransformer transformer) {
+ initManagementContext();
try {
+ highAvailabilityMode = HighAvailabilityMode.MASTER;
--- End diff --
could this go to `HOT_STANDBY`, just in case of accidental writes?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Copy-state command fixes
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/248#discussion_r18969129
--- Diff: usage/launcher/src/main/java/brooklyn/launcher/BrooklynLauncher.java ---
@@ -449,35 +453,65 @@ public BrooklynLauncher haHeartbeatPeriod(Duration val) {
return this;
}
- public BrooklynMementoRawData retrieveState() {
- initManagementContext();
+ public void copyPersistedState(String destinationDir, String destinationLocation) {
+ copyPersistedState(destinationDir, destinationLocation, null);
+ }
+ public void copyPersistedState(String destinationDir, String destinationLocation, CompoundTransformer transformer) {
+ initManagementContext();
try {
+ highAvailabilityMode = HighAvailabilityMode.MASTER;
initPersistence();
} catch (Exception e) {
handleSubsystemStartupError(ignorePersistenceErrors, "persistence", e);
}
+ ManagementPlaneSyncRecord planeState = managementContext.getHighAvailabilityManager().getManagementPlaneSyncState();
+ BrooklynMementoRawData memento = retrieveState();
+ BrooklynMementoRawData newMemento = memento;
+ if (transformer != null) {
+ try {
+ newMemento = transformer.transform(memento);
+ } catch (Exception e) {
+ throw Throwables.propagate(e);
+ }
+ }
+ persistState(newMemento, planeState, destinationDir, destinationLocation);
+
+ }
+
+ /** @deprecated since 0.7.0 use {@link #copyPersistedState} instead */
+ // Make private after deprecation
+ @Deprecated
+ public BrooklynMementoRawData retrieveState() {
+ initManagementContext();
try {
return managementContext.getRebindManager().retrieveMementoRawData();
-
+
} catch (Exception e) {
Exceptions.propagateIfFatal(e);
- LOG.debug("Error rebinding to persisted state (rethrowing): "+e, e);
- throw new FatalRuntimeException("Error rebinding to persisted state: "+
- Exceptions.collapseText(e), e);
+ LOG.debug("Error rebinding to persisted state (rethrowing): " + e, e);
+ throw new FatalRuntimeException("Error rebinding to persisted state: " +
+ Exceptions.collapseText(e), e);
}
}
+ /** @deprecated since 0.7.0 use {@link #copyPersistedState} instead */
+ // Make private after deprecation
+ @Deprecated
public void persistState(BrooklynMementoRawData memento, String destinationDir, String destinationLocation) {
+ persistState(memento, null, destinationDir, destinationLocation);
+ }
+
+ private void persistState(BrooklynMementoRawData memento, ManagementPlaneSyncRecord planeRecord,
+ String destinationDir, String destinationLocation) {
--- End diff --
call it `optionalDestinationLocation` ?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Copy-state command fixes
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/248#discussion_r18926986
--- Diff: usage/launcher/src/main/java/brooklyn/launcher/BrooklynLauncher.java ---
@@ -449,35 +453,65 @@ public BrooklynLauncher haHeartbeatPeriod(Duration val) {
return this;
}
- public BrooklynMementoRawData retrieveState() {
- initManagementContext();
+ public void copyPersistedState(String destinationDir, String destinationLocation) {
+ copyPersistedState(destinationDir, destinationLocation, null);
+ }
+ public void copyPersistedState(String destinationDir, String destinationLocation, CompoundTransformer transformer) {
+ initManagementContext();
try {
+ highAvailabilityMode = HighAvailabilityMode.MASTER;
--- End diff --
HA mode required to trigger load of `ManagementNodeSyncRecord`.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Copy-state command fixes
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/248#discussion_r18969715
--- Diff: core/src/main/java/brooklyn/management/ha/ManagementPlaneSyncRecordPersisterToObjectStore.java ---
@@ -291,6 +291,15 @@ public void waitForWritesCompleted(Duration timeout) throws InterruptedException
masterWriter.waitForCurrentWrites(timeout);
}
+ public void checkpoint(ManagementPlaneSyncRecord record) {
+ init();
+ for (ManagementNodeSyncRecord node : record.getManagementNodes().values()) {
+ if (node.getNodeId() != null) {
--- End diff --
that seems reasonable. `id` might not be null in future, maybe better to filter based on `INITIALIZING` state (or either, i.e. that or `id==null` triggers exclusion)
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Copy-state command fixes
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/248#discussion_r19026297
--- Diff: usage/rest-server/src/test/java/brooklyn/rest/util/json/BrooklynJacksonSerializerTest.java ---
@@ -77,7 +79,7 @@ public String toString() {
}
@Test
- public void testCustomSerializerWithSerializableSillyManagementExample() throws JsonGenerationException, JsonMappingException, IOException {
+ public void testCustomSerializerWithSerializableSillyManagementExample() throws IOException {
--- End diff --
Personal preference: all test methods to say `throws Exception`. We're not testing the compiler, so might as well reduce amount of refactoring we may have to do in the future. It's also more concise.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Copy-state command fixes
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/248#issuecomment-59395253
code changes look good. some minor comments. still need to test tho.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Copy-state command fixes
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-brooklyn/pull/248
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Copy-state command fixes
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/248#discussion_r18926961
--- Diff: core/src/main/java/brooklyn/management/ha/ManagementPlaneSyncRecordPersisterToObjectStore.java ---
@@ -291,6 +291,15 @@ public void waitForWritesCompleted(Duration timeout) throws InterruptedException
masterWriter.waitForCurrentWrites(timeout);
}
+ public void checkpoint(ManagementPlaneSyncRecord record) {
+ init();
+ for (ManagementNodeSyncRecord node : record.getManagementNodes().values()) {
+ if (node.getNodeId() != null) {
--- End diff --
Null check included in case the node in the memento is the one being initialised by `BrooklynLauncher` in the copy state command (whose state is `INITIALIZING` and whose id is `null`). Is there a better way to handle this case?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Copy-state command fixes
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/248#discussion_r18969362
--- Diff: usage/launcher/src/main/java/brooklyn/launcher/BrooklynLauncher.java ---
@@ -449,35 +453,65 @@ public BrooklynLauncher haHeartbeatPeriod(Duration val) {
return this;
}
- public BrooklynMementoRawData retrieveState() {
- initManagementContext();
+ public void copyPersistedState(String destinationDir, String destinationLocation) {
+ copyPersistedState(destinationDir, destinationLocation, null);
+ }
+ public void copyPersistedState(String destinationDir, String destinationLocation, CompoundTransformer transformer) {
+ initManagementContext();
try {
+ highAvailabilityMode = HighAvailabilityMode.MASTER;
initPersistence();
} catch (Exception e) {
handleSubsystemStartupError(ignorePersistenceErrors, "persistence", e);
}
+ ManagementPlaneSyncRecord planeState = managementContext.getHighAvailabilityManager().getManagementPlaneSyncState();
+ BrooklynMementoRawData memento = retrieveState();
+ BrooklynMementoRawData newMemento = memento;
+ if (transformer != null) {
+ try {
+ newMemento = transformer.transform(memento);
+ } catch (Exception e) {
+ throw Throwables.propagate(e);
+ }
+ }
+ persistState(newMemento, planeState, destinationDir, destinationLocation);
+
+ }
+
+ /** @deprecated since 0.7.0 use {@link #copyPersistedState} instead */
+ // Make private after deprecation
+ @Deprecated
+ public BrooklynMementoRawData retrieveState() {
+ initManagementContext();
try {
return managementContext.getRebindManager().retrieveMementoRawData();
-
+
} catch (Exception e) {
Exceptions.propagateIfFatal(e);
- LOG.debug("Error rebinding to persisted state (rethrowing): "+e, e);
- throw new FatalRuntimeException("Error rebinding to persisted state: "+
- Exceptions.collapseText(e), e);
+ LOG.debug("Error rebinding to persisted state (rethrowing): " + e, e);
+ throw new FatalRuntimeException("Error rebinding to persisted state: " +
+ Exceptions.collapseText(e), e);
}
}
+ /** @deprecated since 0.7.0 use {@link #copyPersistedState} instead */
+ // Make private after deprecation
+ @Deprecated
public void persistState(BrooklynMementoRawData memento, String destinationDir, String destinationLocation) {
+ persistState(memento, null, destinationDir, destinationLocation);
+ }
+
+ private void persistState(BrooklynMementoRawData memento, ManagementPlaneSyncRecord planeRecord,
--- End diff --
call it `copyPersistedState` for consistency and to avoid confusion with the normal persistence mechanism?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Copy-state command fixes
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/248#discussion_r19026035
--- Diff: usage/launcher/src/main/java/brooklyn/launcher/BrooklynLauncher.java ---
@@ -449,35 +453,96 @@ public BrooklynLauncher haHeartbeatPeriod(Duration val) {
return this;
}
- public BrooklynMementoRawData retrieveState() {
- initManagementContext();
+ /**
+ * @param destinationDir Directory for state to be copied to
+ */
+ public void copyPersistedState(String destinationDir) {
+ copyPersistedState(destinationDir, null, null);
+ }
+ /**
+ * @param destinationDir Directory for state to be copied to
+ * @param destinationLocation Optional location if target for copied state is a blob store.
+ */
+ public void copyPersistedState(String destinationDir, @Nullable String destinationLocation) {
+ copyPersistedState(destinationDir, destinationLocation, null);
+ }
+
+ /**
+ * @param destinationDir Directory for state to be copied to
+ * @param destinationLocation Optional location if target for copied state is a blob store.
+ * @param transformer Optional transformations to apply to retrieved state before it is copied.
+ */
+ public void copyPersistedState(String destinationDir, @Nullable String destinationLocation, @Nullable CompoundTransformer transformer) {
+ initManagementContext();
try {
+ highAvailabilityMode = HighAvailabilityMode.HOT_STANDBY;
initPersistence();
} catch (Exception e) {
handleSubsystemStartupError(ignorePersistenceErrors, "persistence", e);
}
+ ManagementPlaneSyncRecord planeState = managementContext.getHighAvailabilityManager().getManagementPlaneSyncState();
+ BrooklynMementoRawData memento = retrieveState(false);
+ BrooklynMementoRawData newMemento = memento;
+ if (transformer != null) {
+ try {
+ newMemento = transformer.transform(memento);
+ } catch (Exception e) {
+ throw Throwables.propagate(e);
--- End diff --
Better to use `throw Exceptions.propagate(e)` so that it handles `InterruptedException` correctly.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Copy-state command fixes
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/248#issuecomment-59327799
Not sure about the test failure:
```
java.io.FileNotFoundException: /home/jenkins/jenkins-slave/workspace/incubator-brooklyn-pull-requests/usage/launcher/pom.xml (No such file or directory)
```
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Copy-state command fixes
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/248#issuecomment-59572833
Merging.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Copy-state command fixes
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/248#issuecomment-59535966
@sjcorbett looks good. Only two extremely minor comments. Will wait for apache-jenkins to confirm, before merging.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Copy-state command fixes
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/248#issuecomment-59531818
@ahgittin Comments should be addressed. Tested in `BrooklynLauncherRebindTestToFiles`. Also fixed `BrooklynJacksonSerializerTest` as a bonus.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---