search

perftools / xhgui

Web interface for XHProf profiling data can store data in MongoDB or PDO database
1.65k stars 343 forks source link

[0.19]: change value of parameter "w" from 0 to 1 in XHGui\Saver\MongoSaver.save #358

Open ym2048 opened 4 years ago

ym2048 commented 4 years ago

change value of parameter "w" from 0 to 1 in XHGui\Saver\MongoSaver.save,so that function will return clear tip when insert data to mongodb failed.

glensc commented 4 years ago

does this change return value? does it make used call to throw on failure?

glensc commented 4 years ago

Seems the answer is "makes code to throw"

as the pipeline fails now with duplicate key errors:

1) XHGui\Test\Searcher\MongoTest::testCustomQuery
MongoDuplicateKeyException: E11000 duplicate key error collection: test_xhprof.results index: _id_ dup key: { : ObjectId('aaaaaaaaaaaaaaaaaaaaaaaa') }
ym2048 commented 4 years ago

Sorry, the reply is a little late

The official document describes the value of the w parameter as follows:

https://www.php.net/manual/zh/mongo.writeconcerns.php w=0: A write will not be followed up with a GLE call, and therefore not checked ("fire and forget") w=1: The write will be acknowledged by the server (the primary on replica set configuration)

If mongodb fails to perform the insert operation, such as primary key conflict, in the case of w= 0, the program will directly return true, and the user will not get any error prompt.

I guess w = 0 is set for performance consideration, but in the debugging stage, it may cause some problems. Maybe the better choice is to parameterize. The value is 0 in debugging and 1 in production phase

ym2048 commented 4 years ago

If you agree with me, I'll be happy to modify the code in this way. Thanks for your time.

glensc commented 4 years ago

self::getLastProfilingId() makes no more sense and should be deleted. you can see the discussion here:

it should actually return new value each time when called so the import.php can work for missing id-s.

w=1 can't be used with current code, as the unit test will fail, the unit tests need to be fixed first. likely needs to have support code to check if id already exists, and skip

glensc commented 4 years ago

also, you need to rebase against current master this branch to catch up changes to CI

glensc commented 3 years ago

Revert unintended close: