standardnotes / self-hosted

[Legacy] Self-host your own Standard Notes server for end-to-end encrypted notes and files
https://github.com/standardnotes/server
GNU Affero General Public License v3.0
339 stars 38 forks source link

[BUG] unable to start with dumped sql from docker image syncing-server:stable #43

Closed jackyzy823 closed 3 years ago

jackyzy823 commented 3 years ago

Describe the bug

It's not possible to upgrade to new architecture from docker image syncing-server:stablewith existing data/database, because syncing-server:stable is lack of an important change of column updated_at_timestamp and created_at_timestamp and their indexes in items . migration1 and migration2.

so the structure of items in dumped sql is missing these two columns.

However auth need column updated_at_timestamp in items to work properly. link , and so it raise error. QueryFailedError: Unknown column 'updated_at_timestamp' in 'order clause'

syncing-server-js do not migrate the changes to add these columns , it just defined in init db script. so it will not change existing table which we imported.

When using old sql dump , there's definitely be an error

HERE might be 3 solutions.

  1. add a migration script in syncing-server-js to include these missing changes. which is the best way. Update: these two columns' values must be updated too.
  2. add these sql after dumped sql from syncing-sever:stable. it works for me
    ALTER TABLE `items` ADD COLUMN  `created_at_timestamp` BIGINT NOT NULL;
    ALTER TABLE `items` ADD COLUMN  `updated_at_timestamp` BIGINT NOT NULL;
    CREATE INDEX `updated_at_timestamp` ON `items` (`updated_at_timestamp`);
    CREATE INDEX `user_uuid_and_updated_at_timestamp_and_created_at_timestamp`  ON `items` (`user_uuid`, `updated_at_timestamp`, `created_at_timestamp`);
    UPDATE `items` SET `created_at_timestamp` = UNIX_TIMESTAMP(`created_at`) * 1000000;
    UPDATE `items` SET `updated_at_timestamp` = UNIX_TIMESTAMP(`updated_at`) * 1000000;

    the last two lines are important for making notes' time right. (let me know if i'm wrong).

3.update syncing-server:stable to syncing-server:latest then dump database then import to new architecture. Not tested. Update: Not work . since syncing-server:latest only add these columns , but not update these columns' value. and in syncing-server:latest these two columns could be null and could work properly, but not work under syncing-server-js because it assume these two columns as NOT NULL. Error Log:

syncing-server-js_1         | TypeError: Cannot read property 'toString' of null
syncing-server-js_1         |     at Timer.convertMicrosecondsToStringDate (/var/www/node_modules/@standardnotes/time/dist/Domain/Time/Timer.js:35:49)
syncing-server-js_1         |     at ItemProjector.projectFull (/var/www/dist/src/Domain/Item/ItemProjector.js:38:36)
syncing-server-js_1         |     at /var/www/dist/src/Domain/Item/SyncResponse/SyncResponseFactory20200115.js:25:94
syncing-server-js_1         |     at Array.map (<anonymous>)
syncing-server-js_1         |     at SyncResponseFactory20200115.createResponse (/var/www/dist/src/Domain/Item/SyncResponse/SyncResponseFactory20200115.js:25:63)
syncing-server-js_1         |     at ItemsController.sync (/var/www/dist/src/Controller/ItemsController.js:65:14)
syncing-server-js_1         |     at processTicksAndRejections (node:internal/process/task_queues:94:5)

Logs

auth_1                      | QueryFailedError: Unknown column 'updated_at_timestamp' in 'order clause'
auth_1                      |     at new QueryFailedError (/var/www/node_modules/typeorm/error/QueryFailedError.js:11:28)
auth_1                      |     at Query.onResult (/var/www/node_modules/typeorm/driver/mysql/MysqlQueryRunner.js:216:45)
auth_1                      |     at Query.execute (/var/www/node_modules/mysql2/lib/commands/command.js:30:14)
auth_1                      |     at PoolConnection.handlePacket (/var/www/node_modules/mysql2/lib/connection.js:425:32)
auth_1                      |     at PacketParser.onPacket (/var/www/node_modules/mysql2/lib/connection.js:75:12)
auth_1                      |     at PacketParser.executeStart (/var/www/node_modules/mysql2/lib/packet_parser.js:75:16)
auth_1                      |     at Socket.<anonymous> (/var/www/node_modules/mysql2/lib/connection.js:82:25)
auth_1                      |     at Socket.emit (node:events:378:20)
auth_1                      |     at Socket.EventEmitter.emit (node:domain:470:12)
auth_1                      |     at addChunk (node:internal/streams/readable:313:12) {
auth_1                      |   code: 'ER_BAD_FIELD_ERROR',
auth_1                      |   errno: 1054,
auth_1                      |   sqlState: '42S22',
auth_1                      |   sqlMessage: "Unknown column 'updated_at_timestamp' in 'order clause'",
auth_1                      |   query: 'SELECT * FROM items WHERE content_type = "SF|MFA" ORDER BY updated_at_timestamp ASC',
auth_1                      |   parameters: []
auth_1                      | }
auth_1                      | error Command failed with exit code 1.
JaspalSuri commented 3 years ago

Hi @jackyzy823, sorry to hear that. Thank you for the report; I've assigned this thread to the dev team for further review. 🙂

karolsojko commented 3 years ago

Hi @jackyzy823

Thanks for this very helpful input - awesome stuff.

At some point of the legacy syncing server life we've ditched the stable tag in favor of latest: https://github.com/standardnotes/syncing-server/issues/162#issuecomment-763445329

You can see that production releases were done with latest: https://github.com/standardnotes/syncing-server/blob/master/.github/workflows/prod.yml#L54

The two migrations you've mentioned were included in the last available production version of the syncing server thus the data would've gotten the timestamps columns treatment you mentioned.

Does that solve your issue?

jackyzy823 commented 3 years ago

The two migrations you've mentioned were included in the last available production version of the syncing server thus the data would've gotten the timestamps columns treatment you mentioned.

Is syncing server which you mentioned syncing-server:latest (ruby) ? In syncing-server:latest these columns is NULL by default . items created after this change will set these columns , but old items will still be NULL . If i want to move to syncing-sever-js , that will cause syncing-sever-js raise error Cannot read property 'toString' of null

karolsojko commented 3 years ago

@jackyzy823 this should be fixed with syncing-server-js:1.38.2

I've updated the standalone to include that version. Could you do ./server.sh update and check again if that migration process works for you from scratch?

jackyzy823 commented 3 years ago

Maybe no.

const itemsTableExists = itemsTableExistsQueryResult[0].count === '1'

and

 const updatedAtTimestampColumnExists = updatedAtTimestampColumnExistsQueryResult[0].count === '1'

type of itemsTableExistsQueryResult[0].count and updatedAtTimestampColumnExistsQueryResult[0].count is number . so the following logic will never be executed.

karolsojko commented 3 years ago

good catch @jackyzy823

fixed and updated standalone with the latest version. I've tested on a db dump without the mentioned columns and seems to work fine - can you confirm and close if it's ok?

jackyzy823 commented 3 years ago

It works. Thanks for your work!