Skip to content
This repository was archived by the owner on Dec 20, 2023. It is now read-only.

Fix getMongodStartedExpression for newer versions#40

Open
mkg20001 wants to merge 3 commits into
mongodb-js:masterfrom
mkg20001:patch-1
Open

Fix getMongodStartedExpression for newer versions#40
mkg20001 wants to merge 3 commits into
mongodb-js:masterfrom
mkg20001:patch-1

Conversation

@mkg20001
Copy link
Copy Markdown

@mkg20001 mkg20001 commented Jun 27, 2017

Newer versions of mongod don't print "wating for connections" (or it's now a debug message)

This line seems to indicate mongod is ready

Fixes #45

Newer versions of mongod don't print "wating for connections" (or it's now a debug message)

This line seems to indicate mongod is ready
@winfinit
Copy link
Copy Markdown
Contributor

winfinit commented Jun 27, 2017 via email

@mkg20001
Copy link
Copy Markdown
Author

Oh, didn't think about that.
Maybe that should be dynamic based on the version. I just don't know at which version the regex broke.
(And I also have never written typescript)

@winfinit
Copy link
Copy Markdown
Contributor

winfinit commented Jun 27, 2017 via email

@StarpTech
Copy link
Copy Markdown

StarpTech commented Aug 13, 2017

@winfinit This would also fix #41 but it seems that it is not consistent because on windows the old message is still there waiting for connections on port but on linux only \[initandlisten\] setting featureCompatibilityVersion is logged

@StarpTech
Copy link
Copy Markdown

StarpTech commented Aug 13, 2017

@mkg20001 where did you get this information? When its correct we should check for both

@StarpTech
Copy link
Copy Markdown

StarpTech commented Aug 13, 2017

I found it.

Windows
https://github.com/mongodb/mongo/blob/477a40cc79c11ac18839997daf4301c2e4c748b1/src/mongo/util/net/listen.cpp#L244

Linux
https://github.com/mongodb/mongo/blob/477a40cc79c11ac18839997daf4301c2e4c748b1/src/mongo/util/net/listen.cpp#L272

there are two different logs which indicates that the server is ready. @mkg20001 please add both messages and we are save.

@mkg20001
Copy link
Copy Markdown
Author

Done

Comment thread src/mongod-helper.ts Outdated
let log: string = message.toString();

let mongodStartExpression: RegExp = this.getMongodStartedExpression();
let mongodStartExpression2: RegExp = this.getMongodStartedExpression2();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Call it by the name Linux, Win32

@StarpTech
Copy link
Copy Markdown

BTW: to check that mongodb is running with this method is really bad we should check this with a db command in certain intervals.

@StarpTech
Copy link
Copy Markdown

Hi @winfinit could you verify it and merge it ?

Comment thread src/mongod-helper.ts

getMongodStartedExpression2(): RegExp {
return /\[initandlisten\] setting featureCompatibilityVersion/i;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Couldn't we use a single regexp with the two expressions, rather than two different functions ?
Something like /(waiting for connections on port|\[initandlisten\] setting featureCompatibilityVersion)/i

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Due to the performance it has no significant impact but it's more readable. The variable names could be renamed.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I agree, parsing logs with regexp is pretty bad performance-wise anyway. And getMongodStartedExpression2 is not a very descriptive function name, I only suggested that for readability.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Yes, this should be renamed.

@StarpTech
Copy link
Copy Markdown

@winfinit please review

@StarpTech
Copy link
Copy Markdown

StarpTech commented Sep 11, 2017

@winfinit it would be great if you have a look.

@StarpTech
Copy link
Copy Markdown

@winfinit ???

@mkg20001
Copy link
Copy Markdown
Author

@winfinit ??

@hems
Copy link
Copy Markdown

hems commented Dec 27, 2018

??

@csprocket777
Copy link
Copy Markdown

Was this ever resolved? This is a blocker for me currently.

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.

Please merge PR to support newer mongodb versions

6 participants