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

Fix getMongodStartedExpression for newer versions #40

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

mkg20001
Copy link

@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
Contributor

winfinit commented Jun 27, 2017 via email

@mkg20001
Copy link
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
Contributor

winfinit commented Jun 27, 2017 via email

@StarpTech
Copy link

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

StarpTech commented Aug 13, 2017

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

@StarpTech
Copy link

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
Author

Done

stdoutHandler(message: string | Buffer): void {
this.debug(`mongod stdout: ${message}`);
let log: string = message.toString();

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

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

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

Hi @winfinit could you verify it and merge it ?

@@ -76,6 +77,10 @@ export class MongodHelper {
return /waiting for connections on port/i;
}

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

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

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.

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.

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

@winfinit please review

@StarpTech
Copy link

StarpTech commented Sep 11, 2017

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

@StarpTech
Copy link

@winfinit ???

@mkg20001
Copy link
Author

@winfinit ??

@hems
Copy link

hems commented Dec 27, 2018

??

@csprocket777
Copy link

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