Skip to content
This repository has been archived by the owner on Jun 11, 2024. It is now read-only.

Application start takes too much time - Closes #2426 #2485

Merged
merged 5 commits into from
Oct 19, 2018

Conversation

pablitovicente
Copy link
Contributor

What was the problem?

Application startup was too slow due to SQL in migration /db/sql/migrations/runtime.sql

How did I fix it?

@4miners suggested faster queries

How to test it?

  • time node app -n testnet using the old queries (take note of the time)
  • time node app -n testnet using the queries in this PR (take note of the time and compare with previous time)

Review checklist

DROP TABLE mem_accounts2u_delegates;
CREATE TABLE mem_accounts2u_delegates AS TABLE mem_accounts2delegates;
ALTER TABLE mem_accounts2u_delegates ADD CONSTRAINT "mem_accounts2u_delegates_accountId_fkey" FOREIGN KEY ("accountId") REFERENCES mem_accounts(address) ON DELETE CASCADE;
CREATE INDEX "mem_accounts2u_delegates_accountId" ON mem_accounts2u_delegates("accountId");

DELETE FROM mem_accounts2u_multisignatures;
INSERT INTO mem_accounts2u_multisignatures ("accountId", "dependentId") SELECT "accountId", "dependentId" FROM mem_accounts2multisignatures;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make the change also for this query.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will though impact was less as it was changing a small number of rows but it makes sense for both queries to look the same

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, just for consistency. Also, there are some unit tests that are not passing after the change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah checking the unit tests but I think they were not passing without the change; Just merged develop back in and will check again.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's because of the change in this PR:

16:50:10        MigrationsRepository
16:50:10          applyRuntime()
16:50:10            should set peers to disconnected mode and clock to NULL for non-banned peers:
16:50:10 
16:50:10       �[31mAssertionError: expected 0 to deeply equal 1�[0m
16:50:10       �[32m+ expected�[0m �[31m- actual�[0m
16:50:10 
16:50:10       �[31m-0�[0m
16:50:10       �[32m+1�[0m
16:50:10       �[0m�[90m
16:50:10     at Context.<anonymous> (test/unit/db/repos/migrations.js:170:38)
16:50:10     at Generator.next (<anonymous>:null:null)
16:50:10     at onFulfilled (node_modules/co/index.js:65:19)
16:50:10     at tryCatcher (node_modules/bluebird/js/release/util.js:16:23)
16:50:10     at Promise._settlePromiseFromHandler (node_modules/bluebird/js/release/promise.js:512:31)
16:50:10     at Promise._settlePromise (node_modules/bluebird/js/release/promise.js:569:18)
16:50:10     at Promise._settlePromise0 (node_modules/bluebird/js/release/promise.js:614:10)
16:50:10     at Promise._settlePromises (node_modules/bluebird/js/release/promise.js:693:18)
16:50:10     at Async._drainQueue (node_modules/bluebird/js/release/async.js:133:16)
16:50:10     at Async._drainQueues (node_modules/bluebird/js/release/async.js:143:10)
16:50:10     at Immediate.Async.drainQueues [as _onImmediate] (node_modules/bluebird/js/release/async.js:17:14)
16:50:10   

db/sql/migrations/runtime.sql Outdated Show resolved Hide resolved
db/sql/migrations/runtime.sql Outdated Show resolved Hide resolved
4miners and others added 2 commits October 19, 2018 09:13
Co-Authored-By: pablitovicente <pvicente@gmail.com>
Co-Authored-By: pablitovicente <pvicente@gmail.com>
db/sql/migrations/runtime.sql Show resolved Hide resolved
ALTER TABLE mem_accounts2u_multisignatures ADD CONSTRAINT "mem_accounts2u_multisignatures_accountId_fkey" FOREIGN KEY ("accountId") REFERENCES mem_accounts(address) ON DELETE CASCADE;
CREATE INDEX IF NOT EXISTS "mem_accounts2u_multisignatures_accountId" ON mem_accounts2u_multisignatures("accountId");


Copy link
Contributor

@nazarhussain nazarhussain Oct 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should not we just disable the indexes on table for data import and then enable them back and reindex the table, instead of this approach?

UPDATE pg_index
SET indisready=false
WHERE indrelid = (
    SELECT oid
    FROM pg_class
    WHERE relname='<TABLE_NAME>'
);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same for constraints...

ALTER TABLE TableName NOCHECK CONSTRAINT ConstraintName;

and later

ALTER TABLE TableName CHECK CONSTRAINT ConstraintName;

@nazarhussain
Copy link
Contributor

As discussed with @4miners the above approach will be used temperately and later we are not going to need these unconfirmed tables. Then will remove this logic completely.

@shuse2 shuse2 merged commit e64755e into development Oct 19, 2018
@shuse2 shuse2 deleted the 2426-application-start-takes-too-much-time branch October 19, 2018 11:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Application start takes too much time
4 participants