Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Issue #19296 - Cleanup old unused code/files and consolidate #224

Merged
merged 5 commits into from

3 participants

@bendiy
Owner

This pull request will change node-datasource to only use XM.SimpleModel.
We can now cleanup the old XT code which this pull request does.

Once this has been merged, you will need to make some changes to your server.

  1. Review changes to sample_config.js file and update your local config.js file to match. Several setting hard coded into main.js have been moved here:

    https://github.com/bendiy/xtuple/blob/19296/node-datasource/sample_config.js

  2. You may need to remove all active sessions from your database because the session secret is now stored in xtuple/node-datasource/lib/private/salt.txt

    DELETE FROM xt.sessionstore;

  3. All npm package installs have been consolidated to xtuple/node_modules. You need to delete your old node_modues directories:

    cd xtuple/node-datasource
    rm -rf node_modules
    cd xtuple/node-datasource/xt
    rm -rf node_modules
    cd xtuple/lib/backbone-x
    rm -rf node_modules

  4. Then install all npm packages which appears to require sudo now:

    cd xtuple
    sudo npm install

The salt.txt file is currently under revision control. Should it be removed?

https://github.com/xtuple/xtuple/blob/master/node-datasource/lib/private/salt.txt

I found the cause of file permissions changing from 664 to 775. If you are using SAMBA to access source code, you need to make this change to your /etc/samba/smb.conf file.

http://stackoverflow.com/a/13020343/251019

Then be sure to restart SAMBA:

sudo service smbd restart
sudo service nmbd restart

All file permissions have been cleaned up in this pull request.

I've move the pg pool size to the config file and changed it from 12 to 15. npm pg package was also upgraded to 0.14.1 which addresses some issues with pgpool and improves memory usage:

brianc/node-postgres#227

You will see a warning when running the installer. It is from these issues and AFAIK not in place yet, but will be in v1.0.0 of the pg package:

brianc/node-postgres#301
brianc/node-postgres#271

WARNING!!
parsing and returning floats from PostgreSQL server is deprecated
JavaScript has a hard time with floats and there is precision loss which can cause
unexpected, hard to trace, potentially bad bugs in your program
for more information see the following:
https://github.com/brianc/node-postgres/pull/271
in node-postgres v1.0.0 all floats & decimals will be returned as strings

I've split the pg.connect and pg.client.query code into a child process. This runs all Postgres queries in their own process and frees up the main.js node process to do more work. The pgworker.js process can consume about 20% CPU usage on it's own CPU, letting main.js be 20% faster.

My benchmarks show ~60% performance increase on Apache Bench and ~30% performance increase using autobench httperf from the #19296 code when using a child process pgWorker.
Autobench httperf requests/sec:
pgPoolSize = 3

Apache Bench Response Time
pgPoolSize = 3

@shackbarth shackbarth merged commit 33722fa into from
@davecramer

I'm confused, the pg pool appears to default to 1 now not 13, or 15 ?

@bendiy
Owner

The default of 15 is in the sample config. If that it not set, it defaults to one here:
https://github.com/xtuple/xtuple/blob/master/node-datasource/xt/database/database.js#L15

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
13 node-datasource/lib/ext/datasource.js
@@ -36,6 +36,17 @@ white:true*/
callback(m.err, m.result);
});
+
+ this.worker.on('exit', function (code, signal) {
+ var pid = that.worker.pid,
+ exitCode = that.worker.exitCode,
+ signalCode = that.worker.signalCode;
+
+ X.err('pgWorker ' + pid + ' died (exitCode: ' + exitCode + ' signalCode: ' + signalCode + '). Cannot run any more queries.');
+
+ // TODO - Figure out how to restart the worker. This doesn't work.
+ //that.worker = require('child_process').fork(__dirname + '/pgworker.js');
+ });
}
// NOTE: Round robin benchmarks are slower then the above single pgworker code.
@@ -103,7 +114,7 @@ white:true*/
/**
* Connected.
*
- * NOIE: This is only used when not using a seperate pgWorker process.
+ * NOTE: This is only used when not using a seperate pgWorker process.
* It's useful if you need to run the node-inspector debugger which breaks on multiple processes.
* See: https://github.com/dannycoates/node-inspector/issues/130
* You can also just run, "kill -USR1 12345", to start the debugger on a running process
View
2  node-datasource/xt/database/database.js
@@ -12,7 +12,7 @@ regexp:true, undef:true, strict:true, trailing:true, white:true */
@extends X.Object
*/
X.Database = X.Object.extend(/** @lends X.Database */{
- poolSize: X.options && X.options.datasource & X.options.datasource.pgPoolSize ? X.options.datasource.pgPoolSize : 15,
+ poolSize: X.options && X.options.datasource && (typeof X.options.datasource.pgPoolSize !== 'undefined') ? X.options.datasource.pgPoolSize : 1,
className: "X.Database",
cleanupCompletedEvent: "cleanupCompleted",
Something went wrong with that request. Please try again.