You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by remibergsma <gi...@git.apache.org> on 2016/04/07 19:37:10 UTC
[GitHub] cloudstack pull request: Fix concurrency issue on virtual router
GitHub user remibergsma opened a pull request:
https://github.com/apache/cloudstack/pull/1465
Fix concurrency issue on virtual router
The json files now have UUIDs to prevent them from getting overwritten before they've been executed. Prevents config to be pushed to the wrong router :-s
```
2016-02-25 18:32:23,797 DEBUG [c.c.a.t.Request] (AgentManager-Handler-1:null) (logid:) Seq 2-4684025087442026584: Processing: { Ans: , MgmtId: 90520732674657, via: 2, Ver: v1, Flags: 10, [{"com.cloud.agent.api.routing.GroupA
nswer":{"results":["null - success: null","null - success: [INFO] update_config.py :: Processing incoming file => vm_dhcp_entry.json.4ea45061-2efb-4467-8eaa-db3d77fb0a7b\n[INFO] Processing JSON file vm_dhcp_entry.json.4ea4506
1-2efb-4467-8eaa-db3d77fb0a7b\n"],"result":true,"wait":0}}] }
```
On the router:
```
2016-02-25 18:32:23,416 merge.py __moveFile:298 Processed file written to /var/cache/cloud/processed/vm_dhcp_entry.json.4ea45061-2efb-4467-8eaa-db3d77fb0a7b.gz
```
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/remibergsma/cloudstack 47_fix_concurrency_issue
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/1465.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 #1465
----
commit f632215b0493b210201fa7590db00ce64cf25e4a
Author: Miguel Ferreira <mi...@me.com>
Date: 2016-02-25T13:32:09Z
Make the generated json files unique to prevent concurrency issues
Backported from Cosmic.
----
---
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] cloudstack pull request: Fix concurrency issue on virtual router
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1465#discussion_r58927993
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java ---
@@ -300,6 +303,8 @@ public ExecutionResult createFileInVR(final String routerIp, final String path,
final File permKey = new File("/root/.ssh/id_rsa.cloud");
String error = null;
+ s_logger.debug("Creating file in VR " + filename);
--- End diff --
Please wrap in a ``if (s_logger.isDebugEnabled())`` block to avoid unnecessary string concatenation when not logging at ``DEBUG`` level.
---
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] cloudstack pull request: Fix concurrency issue on virtual router
Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/1465#issuecomment-207051489
@DaanHoogland This can be pointed towards 4.7 so I will close this and open one to 4.7 instead of master.
---
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] cloudstack pull request: Fix concurrency issue on virtual router
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1465#issuecomment-207044590
code LGTM running integration suite
---
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] cloudstack pull request: Fix concurrency issue on virtual router
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1465#discussion_r58923375
--- Diff: systemvm/patches/debian/config/opt/cloud/bin/vr_cfg.sh ---
@@ -91,7 +91,7 @@ do
done < $cfg
#remove the configuration file, log file should have all the records as well
-rm -f $cfg
+mv $cfg /var/cache/cloud/processed/
--- End diff --
Do we know this will get cleaned?
---
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] cloudstack pull request: Fix concurrency issue on virtual router
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1465#discussion_r58927692
--- Diff: core/src/com/cloud/agent/resource/virtualnetwork/facade/AbstractConfigItemFacade.java ---
@@ -104,13 +109,25 @@ public static AbstractConfigItemFacade getInstance(final Class<? extends Network
return instance;
}
+
+ private static String appendUuidToJsonFiles(final String filename) {
+ String remoteFileName = new String(filename);
+ if (remoteFileName.endsWith("json")) {
+ remoteFileName += "." + UUID.randomUUID().toString();
+ }
+ return remoteFileName;
+ }
+
protected List<ConfigItem> generateConfigItems(final ConfigBase configuration) {
final List<ConfigItem> cfg = new LinkedList<>();
- final ConfigItem configFile = new FileConfigItem(VRScripts.CONFIG_PERSIST_LOCATION, destinationFile, gson.toJson(configuration));
+ final String remoteFilename = appendUuidToJsonFiles(destinationFile);
+ s_logger.debug("Transformed filename " + destinationFile + " to " + remoteFilename);
--- End diff --
Please wrap in a ``if (s_logger.isDebugEnabled())`` block to avoid unnecessary string concatenation when not logging at ``DEBUG`` level.
---
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] cloudstack pull request: Fix concurrency issue on virtual router
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1465#issuecomment-207054605
it was already started :(
---
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] cloudstack pull request: Fix concurrency issue on virtual router
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1465#discussion_r58927653
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java ---
@@ -292,6 +292,9 @@ public ExecutionResult executeInVR(final String routerIp, final String script, f
if (details == null) {
details = parser.getLines();
}
+
+ s_logger.debug("Executing script in VR " + script);
--- End diff --
Please wrap in a ``if (s_logger.isDebugEnabled())`` block to avoid unnecessary string concatenation when not logging at ``DEBUG`` level.
---
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] cloudstack pull request: Fix concurrency issue on virtual router
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1465#discussion_r58927425
--- Diff: core/src/com/cloud/agent/resource/virtualnetwork/facade/AbstractConfigItemFacade.java ---
@@ -104,13 +109,25 @@ public static AbstractConfigItemFacade getInstance(final Class<? extends Network
return instance;
}
+
+ private static String appendUuidToJsonFiles(final String filename) {
+ String remoteFileName = new String(filename);
--- End diff --
Why are is a ``new String(filename)`` being done? This avoids the constant pool, and is extremely expensive. Consider the following usage of ``StringBuilder`` to avoid unnecessary pressure on the ``String`` constant pool due to the ``new`` call and string concatenation:
```
final StringBuilder remoteFileName = new StringBuilder(filename);
if (filename.endWith("json")) {
remoteFileName.append(".");
remoteFileName.append(UUID.randomUUID()); // implicit call to toString
}
return remoteFileName.toString();
```
---
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] cloudstack pull request: Fix concurrency issue on virtual router
Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma closed the pull request at:
https://github.com/apache/cloudstack/pull/1465
---
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.
---