8000 implement a global deadline when running testcode in the local arangosh by dothebart · Pull Request #11123 · arangodb/arangodb · GitHub
[go: up one dir, main page]

Skip to content

implement a global deadline when running testcode in the local arangosh #11123

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account < 10000 /p>

Merged
merged 46 commits into from
Mar 13, 2020

Conversation

dothebart
Copy link
Contributor
@dothebart dothebart commented Feb 18, 2020

this enables us to have a timeout for testfiles that we eval' into the testing.js arangosh by runInLocalArangosh.

… in the local arangosh that also runs the test framework
@dothebart dothebart requested a review from ObiWahn February 18, 2020 22:40
@jsteemann jsteemann added this to the devel milestone Feb 19, 2020
ObiWahn and others added 8 commits February 19, 2020 16:10
…n-local-arangosh

* origin/devel:
  Move subqueries down (#11135)
  Update OskarTestSuitesBlackList (#11132)
  fix shrink method in aqlitemblock (#11122)
The mixture of double (seconds), double (millisconds),
chrono::duration<dobule>, chrono::milliseconds, etc. should be cleaned
up!!!
@ObiWahn ObiWahn force-pushed the feature/timout-run-in-local-arangosh branch from 1128ac7 to 6d222ec Compare February 20, 2020 11:35
@dothebart
Copy link
Contributor Author

The results of this can be observed by specifying a small timeout:
./scripts/unittest shell_client --oneTestTimeout 10

2020-02-20T12:26:59.397Z [============] runInLocalArangosh: Trying tests/js/common/shell/aql-index-usage-rocksdb.js ... 

2020-02-20T12:26:59.400Z [------------] 1 tests from IndexUsageSuite (setUpAll: 0ms)
2020-02-20T12:26:59.400Z [ RUN        ] testIndexUsage
2020-02-20T12:27:09.404Z [   FAILED   ] testIndexUsage (setUp: 101ms, test: 9902ms, tearDown: 1ms)
ArangoError: Timeout for remote!
    at ArangoCollection.getIndexes.ArangoCollection.indexes (/home/willi/src/devel4/js/client/modules/@arangodb/arango-collection.js:671:50)
    at Object.testIndexUsage (tests/js/common/shell/aql-index-usage-rocksdb.js:101:34)
    at Object.run (/home/willi/src/devel4/js/common/modules/jsunity/jsunity.js:549:27)
    at Object.Run [as run] (/home/willi/src/devel4/js/common/modules/jsunity.js:275:24)
    at tests/js/common/shell/aql-index-usage-rocksdb.js:128:9
    at tests/js/common/shell/aql-index-usage-rocksdb.js:131:3
    at RunTest (/home/willi/src/devel4/js/common/modules/jsunity.js:389:12)
    at testFunc (eval at runInLocalArangosh (/home/willi/src/devel4/js/client/modules/@arangodb/test-utils.js:889:3), <anonymous>:4:9)
    at runInLocalArangosh (/home/willi/src/devel4/js/client/modules/@arangodb/test-utils.js:893:18)
    at Object.performTests (/home/willi/src/devel4/js/client/modules/@arangodb/test-utils.js:233:21) - test failed
ArangoError: Timeout for remote!
    at ArangoCollection.drop (/home/willi/src/devel4/js/client/modules/@arangodb/arango-collection.js:510:48)
    at Proxy.ArangoDatabase._drop (/home/willi/src/devel4/js/client/modules/@arangodb/arango-database.js:442:29)
    at Object.tearDown (tests/js/common/shell/aql-index-usage-rocksdb.js:57:10)
    at /home/willi/src/devel4/js/common/modules/jsunity/jsunity.js:483:16
    at Object.run (/home/willi/src/devel4/js/common/modules/jsunity/jsunity.js:554:19)
    at Object.Run [as run] (/home/willi/src/devel4/js/common/modules/jsunity.js:275:24)
    at tests/js/common/shell/aql-index-usage-rocksdb.js:128:9
    at tests/js/common/shell/aql-index-usage-rocksdb.js:131:3
    at RunTest (/home/willi/src/devel4/js/common/modules/jsunity.js:389:12)
    at testFunc (eval at runInLocalArangosh (/home/willi/src/devel4/js/client/modules/@arangodb/test-utils.js:889:3), <anonymous>:4:9) - tearDown failed
2020-02-20T12:27:09.404Z [------------] 1 tests from IndexUsageSuite ran (tearDownAll: 1ms)
2020-02-20T12:27:09.404Z [   PASSED   ] 0 tests.
2020-02-20T12:27:09.404Z [   FAILED   ] 1 tests.
2020-02-20T12:27:09.404Z [============] Ran: 1 tests (0 passed, 1 failed) (10004ms total)

oops! Skipping remaining tests, server is gone.
Thu Feb 20 2020 13:27:09 GMT+0100 (Central European Standard Time) Shutting down...

@dothebart dothebart requested review from KVS85 and jsteemann February 20, 2020 12:30
@dothebart dothebart marked this pull request as ready for review February 20, 2020 12:30
@dothebart dothebart changed the title attempt to implement a global connection timout when running testcode… implement a global connection timout when running testcode in the local arangosh Feb 20, 2020
@dothebart
Copy link
Contributor Author

@dothebart
Copy link
Contributor Author

Copy link
Contributor
@ObiWahn ObiWahn left a comment

Choose a reason for hiding this comment

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

Please remove the code using __cpp_lib_chrono as it is a c++20 feature. The Mac and Windows compilers are not broken, but just support the c++17 standard.

Copy link
Contributor
@ObiWahn ObiWahn left a comment

Choose a reason for hiding this comment

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

I do not understand how you restricted it to arangosh only. Fox example isExecutionDeadlineReached is used in JS_Download which is used in server and client?!

@ObiWahn
Copy link
Contributor
ObiWahn commented Mar 9, 2020

You use setExecutionDeadlineInMS only in the function that is available to the client. But it is not obvious at all that people are not allowed to use this function in the server. You need at least some comment or better move the code altogether to arangosh.

8000

Copy link
Contributor
@ObiWahn ObiWahn left a comment

Choose a reason for hiding this comment

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

You use double, uint64_t and chrono types in this PR. I think chrono should be used for all instances, because it is the only type that encodes the expected time unit.

////////////////////////////////////////////////////////////////////////////////
/// @brief set a point in time after which we will abort external connection
////////////////////////////////////////////////////////////////////////////////
static double executionDeadline = 0.0;
Copy link
Contributor

Choose a reason for hiding this comment

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

double

return delta;
}

std::chrono::milliseconds correctTimeoutToExecutionDeadline(std::chrono::milliseconds timeout) {
Copy link
Contributor

Choose a reason for hiding this comment

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

std::chrono::milliseconds

@dothebart
Copy link
Contributor Author

Again - we may fix the usage of chrono once we do this for js_download. its out of scope for this PR.

@ObiWahn
Copy link
Contributor
ObiWahn commented Mar 9, 2020

Just use chrono for your new code and cast to others as required.

6D47
@dothebart
Copy link
Contributor Author

I (we) did, on all places that already use it. js_download will most probably completely be replaced soon to get rid of that http implementation, at that time the implementation can simply be erased.

@dothebart
Copy link
Contributor Author

tests green.

@dothebart
Copy link
Contributor Author

@dothebart
Copy link
Contributor Author

@dothebart
Copy link
Contributor Author

tests all blue.

dothebart and others added 7 commits March 13, 2020 14:03
Co-Authored-By: Jan <jsteemann@users.noreply.github.com>
Co-Authored-By: Jan <jsteemann@users.noreply.github.com>
Co-Authored-By: Jan <jsteemann@users.noreply.github.com>
Co-Authored-By: Jan <jsteemann@users.noreply.github.com>
Co-Authored-By: Jan <jsteemann@users.noreply.github.com>
@dothebart
Copy link
Contributor Author

@dothebart
Copy link
Contributor Author

@jsteemann jsteemann merged commit f57786d into devel Mar 13, 2020
@jsteemann jsteemann deleted the feature/timout-run-in-local-arangosh branch March 13, 2020 18:13
ObiWahn added a commit that referenced this pull request Mar 16, 2020
…-fix/validation-fixes-and-improvements

* 'devel' of https://github.com/arangodb/arangodb: (25 commits)
  Do not instantiate snipped if not collection is found on the server. (#11281)
  Add entries related to search features in 3.7
  fix bug (#11279)
  Docs: Add DocuBlocks for document validation. (#11228)
  Feature/ngram similarity function (#11276)
  Fixed production check, removed assertion (#11273)
  fix compile warning
  Cluster Metrics (#11234)
  Feature/satellite graphs (#11015)
  fix newly created supervision bug with incremental updates (#11269)
  remove useless std::cout output
  fix yet more compile warnings
  Implement memory detection override. (#11268)
  implement a global deadline when running testcode in the local arangosh (#11123)
  Encryption key rotation (#11080)
  fix compile warnings
  fix compile warnings
  Feature/aql subquery execution block impl execute implementation (#10606)
  missing metrics (#10625)
  Bug fix/supervision server cleanup (#11187)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0