Ft/mongo oplog #433

JianqinWang · 2018-02-28T18:56:10Z

Merge after #425

Based off of https://github.com/scality/Arsenal/blob/master/lib/storage/metadata/bucketclient/LogConsumer.js

ironman-machine · 2018-03-01T18:52:05Z

PR has been updated. Reviewers, please be cautious.

ironman-machine · 2018-03-05T23:09:03Z

PR has been updated. Reviewers, please be cautious.

ironman-machine · 2018-03-06T01:54:16Z

PR has been updated. Reviewers, please be cautious.

ironman-machine · 2018-03-06T18:43:48Z

PR has been updated. Reviewers, please be cautious.

ironman-machine · 2018-03-06T19:09:16Z

PR has been updated. Reviewers, please be cautious.

ironman-machine · 2018-03-07T21:17:26Z

PR has been updated. Reviewers, please be cautious.

ironman-machine · 2018-03-07T21:44:11Z

PR has been updated. Reviewers, please be cautious.

ironman-machine · 2018-03-08T00:27:41Z

PR has been updated. Reviewers, please be cautious.

JianqinWang · 2018-03-08T00:33:49Z

@ironman-machine try

ironman-machine · 2018-03-08T00:33:53Z

Hello @JianqinWang

"try": Success: Try build successfully launched on 'http://ci.ironmann.io/gh/scality/Integration/20071' with the following env. args:

{
    "REPO_NAME": "Arsenal",
    "SCALITY_ARSENAL_BRANCH": "ft/mongo-oplog",
    "DEFAULT_BRANCH": "master",
    "SCALITY_INTEGRATION_BRANCH": "ultron/master"
}

JianqinWang · 2018-03-08T01:02:05Z

@ironman-machine try

ironman-machine · 2018-03-08T01:02:07Z

Hello @JianqinWang

"try": Success: Try build successfully launched on 'http://ci.ironmann.io/gh/scality/Integration/20072' with the following env. args:

{
    "REPO_NAME": "Arsenal",
    "SCALITY_ARSENAL_BRANCH": "ft/mongo-oplog",
    "DEFAULT_BRANCH": "master",
    "SCALITY_INTEGRATION_BRANCH": "ultron/master"
}

dora-korpar · 2018-03-08T01:14:00Z

lib/storage/metadata/mongoclient/LogConsumer.js

+
+const ops = {
+    i: 'put',
+    u: 'put',


should this be get?

The i and u ops in mongo oplogs are insert and update, which I mapped both to put

I'm not sure if mongodb oplogs have any logs corresponding to get actions - these logs are records of the replication that happens in the mongodb replication set itself, so the main actions are insert, update, and delete

Also, Lauren and I never found super official documentation (that was clear enough) for an explanation on oplogs - we ended up using this relatively out-dated blog as our primary source: https://www.compose.com/articles/the-mongodb-oplog-and-node-js/

Do it again human slave!:point_right: :runner: (Oh and the pull request has been updated, by the way.)

ironman-machine · 2018-03-08T01:32:18Z

PR has been updated. Reviewers, please be cautious.

vrancurel · 2018-03-08T01:48:50Z

@ironman-machine try

bennettbuchanan

Nice! 👍

bennettbuchanan · 2018-03-08T01:17:23Z

lib/storage/metadata/mongoclient/LogConsumer.js

+        MongoClient.connect(this.mongoUrl, { replicaSet: 'rs0' },
+        (err, client) => {
+            if (err) {
+                this.logger.error('Unable to connect to MongoDB', err);


err => { error: err }

bennettbuchanan · 2018-03-08T01:33:34Z

lib/storage/metadata/mongoclient/LogConsumer.js

+                recordStream.write(undefined);
+            });
+            recordStream.once('info', info => {
+                recordStream.removeAllListeners('error');


Wondering if this is good to do as the error event not being listened for. Maybe we should add a listener in ListRecordStream?

Was trying to follow the format here as closely as possible:

Arsenal/lib/storage/metadata/bucketclient/LogConsumer.js

Line 126 in 6725013

recordStream.removeAllListeners('error');

@rahulreddy @vrancurel any opinions? XD

It's important to understand why that was put in LogConsumer in the first place. I don't like the idea of removing all error listeners. I believe the reason it was used in RecordStream was because Metadata was returning InternalError when we request the next sequence id but there are no logs available. @jonathan-gramain was the author of that class, can you check with him?

bennettbuchanan · 2018-03-08T01:38:23Z

lib/storage/metadata/mongoclient/LogConsumer.js

+    readRecords(params, cb) {
+        const recordStream = new ListRecordStream(this.logger);
+        const limit = params.limit || 10000;
+        const startIDandSeq = params.startSeq.toString().split('|');


From the JSDoc, this function expects params.startSeq to be a number. If it has a | char, I'm guessing it is actually a string? In this case, no need to use toString here.

It's possible that the last logs use numbers though if the instances were started with a different backend - I think I had the toString() here just in case we got a number

bennettbuchanan · 2018-03-08T01:47:30Z

lib/storage/metadata/mongoclient/LogConsumer.js

+        this.database = 'local';
+        this.mongoUrl = format(
+            'mongodb://%s/local',
+            replicaSetHosts);


Wondering about the need for format, seems like just a template string would do (that way we don't have to import this utility). Maybe there is something about replicaSetHosts which requires this?

I will make a separate commit, since MongoClientInterface also uses this format

ironman-machine · 2018-03-08T01:48:55Z

Hello @vrancurel

"try": Success: Try build successfully launched on 'http://ci.ironmann.io/gh/scality/Integration/20075' with the following env. args:

{
    "REPO_NAME": "Arsenal",
    "SCALITY_ARSENAL_BRANCH": "ft/mongo-oplog",
    "DEFAULT_BRANCH": "master",
    "SCALITY_INTEGRATION_BRANCH": "ultron/master"
}

ironman-machine · 2018-03-08T02:10:52Z

💔 ☔ circleCI test failed.

ironman-machine · 2018-03-08T03:35:46Z

PR has been updated. Reviewers, please be cautious.

ironman-machine · 2018-03-08T03:35:47Z

CONFLICT (add/add): Merge conflict in index.js

JianqinWang · 2018-03-08T03:37:47Z

@ironman-machine try

ironman-machine · 2018-03-08T03:37:49Z

Hello @JianqinWang

"try": Success: Try build successfully launched on 'http://ci.ironmann.io/gh/scality/Integration/20080' with the following env. args:

{
    "REPO_NAME": "Arsenal",
    "SCALITY_ARSENAL_BRANCH": "ft/mongo-oplog",
    "DEFAULT_BRANCH": "master",
    "SCALITY_INTEGRATION_BRANCH": "ultron/master"
}

ironman-machine · 2018-03-08T04:48:33Z

☀️ 👍 circleCI test succeeded!

vrancurel · 2018-03-08T16:28:13Z

@ironman-machine r+

ironman-machine · 2018-03-08T16:28:18Z

Hello @vrancurel

"r+": Success

ironman-machine · 2018-03-08T16:28:18Z

⌛ PR is now pending. CI build url: http://ci.ironmann.io/gh/scality/Integration/20110

ironman-machine · 2018-03-08T16:53:09Z

💔 ☔ circleCI test failed.

ironman-machine · 2018-03-08T17:22:41Z

PR has been updated. Reviewers, please be cautious.

vrancurel · 2018-03-08T17:25:04Z

@ironman-machine r-

ironman-machine · 2018-03-08T17:25:05Z

Hello @vrancurel

"r-": Success

vrancurel · 2018-03-08T17:25:53Z

@ironman-machine try

ironman-machine · 2018-03-08T17:25:56Z

Hello @vrancurel

"try": Success: Try build successfully launched on 'http://ci.ironmann.io/gh/scality/Integration/20116' with the following env. args:

{
    "REPO_NAME": "Arsenal",
    "SCALITY_ARSENAL_BRANCH": "ft/mongo-oplog",
    "DEFAULT_BRANCH": "master",
    "SCALITY_INTEGRATION_BRANCH": "ultron/master"
}

vrancurel · 2018-03-08T19:06:16Z

@ironman-machine r+

ironman-machine · 2018-03-08T19:06:20Z

Hello @vrancurel

"r+": Success

ironman-machine · 2018-03-08T19:06:20Z

⌛ PR is now pending. CI build url: http://ci.ironmann.io/gh/scality/Integration/20130

rahulreddy · 2018-03-08T19:33:55Z

lib/storage/metadata/mongoclient/LogConsumer.js

+
+        this.coll = this.db.collection('oplog.rs');
+        return this.coll.find({
+            ns: /^(?!.*metadata.*(?:__)).*metadata\.\w+.*/,


What is this regular expression? Adding a comment of what the expression is trying to filter out would be helpful. Cleaner way would be writing it as a constant on the top of the file

/* ** This is a comment to explain this filter */ const METADATA_FILTER = /^(?!.*metadata.*(?:__)).*metadata\.\w+.*/

and then use it in the code as

return this.coll.find({ ns: METADATA_FILTER, ...

rahulreddy · 2018-03-08T19:34:41Z

lib/storage/metadata/mongoclient/LogConsumer.js

+            awaitData: false,
+            noCursorTimeout: true,
+            OplogReplay: true,
+            numberOfRetries: Number.MAX_VALUE,


Why is retry configured to such a high number?

rahulreddy · 2018-03-08T19:35:11Z

lib/storage/metadata/mongoclient/LogConsumer.js

+                recordStream.write(data);
+            });
+            stream.on('end', () => {
+                recordStream.write(undefined);


Shouldn't this be null to indicate the end of stream?

When we write 'null', it throws this error:

TypeError: May not write null values to stream at validChunk (_stream_writable.js:211:10) at ListRecordStream.Writable.write (_stream_writable.js:245:12) at Cursor.stream.on (/home/jay/Scality/backbeat/node_modules/arsenal/lib/storage/metadata/mongoclient/LogConsumer.js:176:30) at emitNone (events.js:91:20) at Cursor.emit (events.js:185:7) at endReadableNT (_stream_readable.js:974:12) at _combinedTickCallback (internal/process/next_tick.js:74:11) at process._tickCallback (internal/process/next_tick.js:98:9)

ironman-machine · 2018-03-08T20:21:11Z

☀️ 👍 circleCI test succeeded!

JianqinWang force-pushed the ft/mongo-oplog branch from 2c3ca3d to 730d16c Compare March 5, 2018 23:09

JianqinWang force-pushed the ft/mongo-oplog branch from 730d16c to f266013 Compare March 6, 2018 01:54

JianqinWang force-pushed the ft/mongo-oplog branch from f266013 to acd7e19 Compare March 6, 2018 18:43

JianqinWang force-pushed the ft/mongo-oplog branch from 86027cc to 82f06a5 Compare March 6, 2018 19:09

JianqinWang force-pushed the ft/mongo-oplog branch from 82f06a5 to 003af43 Compare March 7, 2018 21:17

JianqinWang force-pushed the ft/mongo-oplog branch from 056c97d to 89b3620 Compare March 8, 2018 00:27

vrancurel previously approved these changes Mar 8, 2018

View reviewed changes

dora-korpar reviewed Mar 8, 2018

View reviewed changes

ploki previously approved these changes Mar 8, 2018

View reviewed changes

JianqinWang force-pushed the ft/mongo-oplog branch from 89b3620 to ebf2c88 Compare March 8, 2018 01:32

vrancurel previously approved these changes Mar 8, 2018

View reviewed changes

bennettbuchanan reviewed Mar 8, 2018

View reviewed changes

JianqinWang closed this Mar 8, 2018

JianqinWang reopened this Mar 8, 2018

JianqinWang added 3 commits March 8, 2018 09:22

ZENKO-15 ft: oplog tailer for MongoDB

a269619

rf: name change for replica set hosts

fa19fc8

rf: remove use of util.format

a9a6b24

JianqinWang force-pushed the ft/mongo-oplog branch from cb69218 to a9a6b24 Compare March 8, 2018 17:22

vrancurel approved these changes Mar 8, 2018

View reviewed changes

rahulreddy reviewed Mar 8, 2018

View reviewed changes

ironman-machine merged commit a9a6b24 into master Mar 8, 2018

ironman-machine added a commit that referenced this pull request Mar 8, 2018

merge #433

189194a

ironman-machine deleted the ft/mongo-oplog branch March 8, 2018 20:21

JianqinWang mentioned this pull request Mar 10, 2018

rf/update mongo oplog #441

Closed

Ft/mongo oplog #433

Ft/mongo oplog #433

Conversation

JianqinWang commented Feb 28, 2018 • edited Loading

ironman-machine commented Mar 1, 2018

ironman-machine commented Mar 5, 2018

ironman-machine commented Mar 6, 2018

ironman-machine commented Mar 6, 2018

ironman-machine commented Mar 6, 2018

ironman-machine commented Mar 7, 2018

ironman-machine commented Mar 7, 2018

ironman-machine commented Mar 8, 2018

JianqinWang commented Mar 8, 2018

ironman-machine commented Mar 8, 2018

JianqinWang commented Mar 8, 2018

ironman-machine commented Mar 8, 2018

Choose a reason for hiding this comment

JianqinWang Mar 8, 2018 • edited Loading

Choose a reason for hiding this comment

JianqinWang Mar 8, 2018 • edited Loading

Choose a reason for hiding this comment

ironman-machine commented Mar 8, 2018

vrancurel commented Mar 8, 2018

bennettbuchanan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ironman-machine commented Mar 8, 2018

ironman-machine commented Mar 8, 2018

ironman-machine commented Mar 8, 2018

ironman-machine commented Mar 8, 2018

JianqinWang commented Mar 8, 2018

ironman-machine commented Mar 8, 2018

ironman-machine commented Mar 8, 2018

vrancurel commented Mar 8, 2018

ironman-machine commented Mar 8, 2018

ironman-machine commented Mar 8, 2018

ironman-machine commented Mar 8, 2018

ironman-machine commented Mar 8, 2018

vrancurel commented Mar 8, 2018

ironman-machine commented Mar 8, 2018

vrancurel commented Mar 8, 2018

ironman-machine commented Mar 8, 2018

vrancurel commented Mar 8, 2018

ironman-machine commented Mar 8, 2018

ironman-machine commented Mar 8, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ironman-machine commented Mar 8, 2018

JianqinWang commented Feb 28, 2018 •

edited

Loading

JianqinWang Mar 8, 2018 •

edited

Loading

JianqinWang Mar 8, 2018 •

edited

Loading