You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by ahgittin <gi...@git.apache.org> on 2014/10/18 08:22:06 UTC
[GitHub] incubator-brooklyn pull request: Bunch of fixes observed during ho...
GitHub user ahgittin opened a pull request:
https://github.com/apache/incubator-brooklyn/pull/255
Bunch of fixes observed during hot standby testing, fixing memory leaks, and tidying API
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ahgittin/incubator-brooklyn misc2
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-brooklyn/pull/255.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 #255
----
commit 6c196dc3f535645a939974c6f80786822d826220
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2014-10-06T03:20:49Z
use a resizable textarea for effector parameters
commit 6c508534260d07e3b1b283eefe17f9d4041c9379
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2014-10-07T04:16:23Z
add better endpoints for /ha/state and /ha/states
commit 5861e2f6af082a2ce8366296f61e80a4c6ba9ed1
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2014-10-07T05:30:22Z
when checking available port, if binding to all interfaces do an explicit check for localhost interface
commit 306b0274247053cb907a3c8183480b763578dac0
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2014-10-07T06:08:14Z
launcher defaults to passwordless for localhost if no security, irrespective of bind address
commit baee8c5d567e3655d6cd678c3dc35f9c26cab9f8
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2014-10-18T00:40:42Z
misc tidies and error checks
commit 81b027b791553bce9e038adc0cdb69eeb3ad1c3b
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2014-10-18T02:41:26Z
comments and rebind-related tidy-up on use of FIRST
commit 8b838c3115dd10050ce3f821edd6e04f963c3327
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2014-10-18T03:00:46Z
fix leak of ssh pool cache and cleanup logic
commit 31f430ebffa7385eab6361eb7ce5d31e5b9a02a4
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2014-10-18T04:48:46Z
more cleanup of leaks and wastage, including SshMachineLocation's sshPoolCache and frequent enrichers which can suppress duplicates, also fix some demotion items, and fix some logic flaws which assumed enrichers would be run, but they might not of if it had been a duplicate, by initializing them with a value which will certainly be changed
commit a4e7741f9658d1117329668cd1ff4581cb5bd32f
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2014-10-18T05:47:20Z
remove finalizers from entity and location, because it causes objects to be kept around a lot longer than desired
commit df5446aa0f12ad0fe2d9f1cd3e2b1bcecea150d2
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2014-10-18T06:11:23Z
fix misc errors during hot standby (access to unavailable methods, clearing more things on location unmanagement) and after rebind (NPE in SshMachineLocation), and fix sensor logic bug in tomcat
----
---
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: Bunch of fixes observed during ho...
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/255#discussion_r19053490
--- Diff: usage/jsgui/src/main/webapp/assets/tpl/apps/param.html ---
@@ -27,6 +27,8 @@
<% if (name == 'locations' || name == 'location') { %>
<td><div id="selector-container" class="input-medium param-value"></div></td>
<% } else { %>
- <td><input type="text" class="input-medium param-value" value="<%= defaultValue %>"></td>
+ <td><!-- use 1 line textarea so it can be resized -->
+ <textarea class="input-medium param-value" style="height: 18px;"><%= _.escape(defaultValue) %></textarea>
--- End diff --
thanks for reminder!
---
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: Bunch of fixes observed during ho...
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/255#discussion_r19053486
--- Diff: core/src/main/java/brooklyn/entity/rebind/BasicEntityRebindSupport.java ---
@@ -156,31 +159,27 @@ public void addFeeds(RebindContext rebindContext, EntityMemento memento) {
} catch (Exception e) {
rebindContext.getExceptionHandler().onAddFeedFailed(entity, feed, e);
}
- } else {
- LOG.warn("Feed not found; discarding feed {} of entity {}({})",
- new Object[] {feedId, memento.getType(), memento.getId()});
- }
-
- if (feed != null) {
+
try {
+ // XXX
--- End diff --
fixed with the real comment, thx
---
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: Bunch of fixes observed during ho...
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/255#discussion_r19052539
--- Diff: core/src/main/java/brooklyn/entity/rebind/BasicEntityRebindSupport.java ---
@@ -156,31 +159,27 @@ public void addFeeds(RebindContext rebindContext, EntityMemento memento) {
} catch (Exception e) {
rebindContext.getExceptionHandler().onAddFeedFailed(entity, feed, e);
}
- } else {
- LOG.warn("Feed not found; discarding feed {} of entity {}({})",
- new Object[] {feedId, memento.getType(), memento.getId()});
- }
-
- if (feed != null) {
+
try {
+ // XXX
--- End diff --
XXX?
---
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: Bunch of fixes observed during ho...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-brooklyn/pull/255
---
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: Bunch of fixes observed during ho...
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/255#discussion_r19052565
--- Diff: usage/jsgui/src/main/webapp/assets/tpl/apps/param.html ---
@@ -27,6 +27,8 @@
<% if (name == 'locations' || name == 'location') { %>
<td><div id="selector-container" class="input-medium param-value"></div></td>
<% } else { %>
- <td><input type="text" class="input-medium param-value" value="<%= defaultValue %>"></td>
+ <td><!-- use 1 line textarea so it can be resized -->
+ <textarea class="input-medium param-value" style="height: 18px;"><%= _.escape(defaultValue) %></textarea>
--- End diff --
I think it's simpler to use `<%- defaultValue %>` (which instructs the templater to escape values) rather than including `_.escape`.
---
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: Bunch of fixes observed during ho...
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/255#discussion_r19053258
--- Diff: core/src/main/java/brooklyn/location/basic/SshMachineLocation.java ---
@@ -219,6 +222,22 @@ public SshMachineLocation(Map properties) {
usedPorts = (usedPorts != null) ? Sets.newLinkedHashSet(usedPorts) : Sets.<Integer>newLinkedHashSet();
}
+ @Override
+ public void init() {
+ super.init();
+ }
+
+ private final transient Object poolCacheMutex = new Object();
+ private LoadingCache<Map<String, ?>, Pool<SshTool>> getSshPoolCache() {
+ synchronized (poolCacheMutex) {
--- End diff --
Regarding @neykov 's comment "Personally prefer double null check (out of the sync block as well) so that we need synchronization on first call only but fine as is."
If you do ever use double-check locking like that, then be sure to declare your field volatile (which then removes much of the performance benefit of the double-check locking). The only time you can get away without the volatile is if it's a primitive (that is 32 bits or less). Otherwise, you risk other threads seeing a semi-initialised object because the writer thread and the second thread don't have a "happens before" relationship.
Double-checked locking is broken (according to the Java memory model spec) in far too many uses of it, but folk "get away with it" (i.e. hardly ever see problems) because of the way the current generations of Intel multi-core chips behave. It's usage is often not worth the performance gain (and consider alternatives like instantiating the field in the constructor, declared final).
But such comments aren't relevant for this particular usage obviously!
---
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: Bunch of fixes observed during ho...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/255#issuecomment-59624884
done! this patches a several leaks, especially after disabling mgmt nodes, fixes a race where sometimes nginx and software process entities might not show running correctly, and several other misc minor tidy-ups.
@grkvlt note https://github.com/ahgittin/incubator-brooklyn/commit/81b027b791553bce9e038adc0cdb69eeb3ad1c3b
ready for merging methinks
---
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: Bunch of fixes observed during ho...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/255#issuecomment-59603772
Finished review. No functional problems found so fine to merge, but worth discussing the comments.
---
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.
---