From 1e772d1427ba02ba9d29fbcffa2e5e0100418be6 Mon Sep 17 00:00:00 2001 From: Wilfried Goesgens Date: Wed, 6 Apr 2016 17:40:28 +0200 Subject: [PATCH 01/20] Adjust to the new reality without make --- README_maintainers.md | 122 +++++++++++++++--------------------------- 1 file changed, 42 insertions(+), 80 deletions(-) diff --git a/README_maintainers.md b/README_maintainers.md index 4670e3832c..4ec75f2b70 100644 --- a/README_maintainers.md +++ b/README_maintainers.md @@ -3,8 +3,8 @@ ArangoDB Maintainers manual =========================== This file contains documentation about the build process, documentation generation means, unittests - put short - if you want to hack parts of arangod this could be interesting for you. -Configure -========= +CMake +===== * *--enable-relative* - relative mode so you can run without make install * *--enable-maintainer-mode* - generate lex/yacc files * *--with-backtrace* - add backtraces to native code asserts & exceptions @@ -26,8 +26,8 @@ At runtime arangod needs to be started with these options: --javascript.v8-options="--gdbjit_dump" --javascript.v8-options="--gdbjit_full" -Debugging the Make process --------------------------- +Debugging the build process +--------------------------- If the compile goes wrong for no particular reason, appending 'verbose=' adds more output. For some reason V8 has VERBOSE=1 for the same effect. Runtime @@ -53,7 +53,8 @@ A sample version to help working with the arangod rescue console may look like t }; print = internal.print; -__________________________________________________________________________________________________________ +HINT: You shouldn't lean on these variables in your foxx services. +______________________________________________________________________________________________________ JSLint ====== @@ -63,19 +64,19 @@ Make target ----------- use - make gitjslint + ./utils/gitjslint.sh to lint your modified files. - make jslint + ./utils/jslint.sh to find out whether all of your files comply to jslint. This is required to make continuous integration work smoothly. -if you want to add new files / patterns to this make target, edit js/Makefile.files +if you want to add new files / patterns to this make target, edit the respective shell scripts. to be safe from committing non-linted stuff add **.git/hooks/pre-commit** with: - make gitjslint + ./utils/jslint.sh Use jslint standalone for your js file @@ -83,15 +84,17 @@ Use jslint standalone for your js file If you want to search errors in your js file, jslint is very handy - like a compiler is for C/C++. You can invoke it like this: - bin/arangosh --jslint js/server/modules/@arangodb/testing.js + bin/arangosh --jslint js/client/modules/@arangodb/testing.js -__________________________________________________________________________________________________________ +_____________________________________________________________________________________________________ ArangoDB Unittesting Framework ============================== Dependencies ------------ - * Ruby, rspec, httparty, boost_test (compile time) +* *Ruby*, *rspec*, *httparty* to install the required dependencies run: + cd UnitTests/HttpInterface; bundler +* boost_test (compile time) Filename conventions @@ -141,7 +144,7 @@ There are several major places where unittests live: HttpInterface - RSpec Client Tests ---------------------------------- +---------------------------------- These tests work on the plain RESTfull interface of arangodb, and thus also test invalid HTTP-requests and so forth, plus check error handling in the server. @@ -158,50 +161,14 @@ arangosh is similar, however, you can only run tests which are intended to be ra require("jsunity").runTest("js/client/tests/shell-client.js"); +mocha tests +----------- +All tests with -spec in their names are using the [mochajs.org](https://mochajs.org) framework. + + jasmine tests ------------- - -Jasmine tests cover two important usecase: - - testing the UI components of aardvark --spec - -aardvark - - - -Invocation methods -================== - -Make-targets ------------- -Most of the tests can be invoked via the main Makefile: (UnitTests/Makefile.unittests) - - unittests - - unittests-brief - - unittests-verbose - - unittests-recovery - - unittests-config - - unittests-boost - - unittests-single - - unittests-shell-server - - unittests-shell-server-only - - unittests-shell-server-aql - - unittests-shell-client-readonly - - unittests-shell-client - - unittests-http-server - - unittests-ssl-server - - unittests-import - - unittests-replication - - unittests-replication-server - - unittests-replication-http - - unittests-replication-data - - unittests-upgrade - - unittests-dfdb - - unittests-foxx-manager - - unittests-dump - - unittests-arangob - - unittests-authentication - - unittests-authentication-parameters - +Jasmine tests cover testing the UI components of aardvark Javascript framework -------------------- @@ -225,7 +192,6 @@ Available choices include: - *all*: (calls multiple) This target is utilized by most of the jenkins builds invoking unit tests. - *single_client*: (see Running a single unittestsuite) - *single_server*: (see Running a single unittestsuite) - - *single_localserver*: (see Running a single unittestsuite) - many more - call without arguments for more details. Passing Options @@ -248,7 +214,7 @@ syntax --option value --sub:option value. Using Valgrind could look like this: - we specify the test to execute - we specify some arangod arguments via --extraargs which increase the server performance - - we specify to run using valgrind (this is supported by all facilities + - we specify to run using valgrind (this is supported by all facilities) - we specify some valgrind commandline arguments Running a single unittestsuite @@ -266,7 +232,7 @@ Testing a single rspec test: scripts/unittest http_server --test api-users-spec.rb **scripts/unittest** is mostly only a wrapper; The backend functionality lives in: -**js/server/modules/@arangodb/testing.js** +**js/client/modules/@arangodb/testing.js** Running foxx tests with a fake foxx Repo ---------------------------------------- @@ -292,8 +258,6 @@ arangod commandline arguments bin/arangod /tmp/dataUT --javascript.unit-tests="js/server/tests/aql-escaping.js" --no-server - make unittest - js/common/modules/loadtestrunner.js __________________________________________________________________________________________________________ @@ -339,7 +303,7 @@ These commands for `-c` mean: If you don't specify them via -c you can also use them in an interactive manner. -__________________________________________________________________________________________________________ +______________________________________________________________________________________________________ Documentation ============= @@ -351,7 +315,7 @@ Dependencies to build documentation: https://pypi.python.org/pypi/setuptools - Download setuptools zip file, extract to any folder, use bundled python 2.6 to install: + Download setuptools zip file, extract to any folder to install: python ez_install.py @@ -361,7 +325,7 @@ Dependencies to build documentation: https://github.com/triAGENS/markdown-pp/ - Checkout the code with Git, use bundled python 2.6 to install: + Checkout the code with Git, use your system python to install: python setup.py install @@ -389,8 +353,8 @@ Dependencies to build documentation: Generate users documentation ============================ -If you've edited REST-Documentation, first invoke `make swagger`. If you've edited examples, see below howto regenerate them. +If you've edited REST-Documentation, first invoke `./utils/generateSwagger.sh`. Run the `make` command in `arangodb/Documentation/Books` to generate it. The documentation will be generated into `arangodb/Documentation/Books/books/Users` - use your favourite browser to read it. @@ -441,24 +405,23 @@ Generate an ePub: Where to add new... ------------------- - - js/action/api/* - markdown comments in source with execution section + - Documentation/DocuBlocks/* - markdown comments with execution section - Documentation/Books/Users/SUMMARY.md - index of all sub documentations - - Documentation/Scripts/generateSwaggerApi.py - list of all sections to be adjusted if generate -------- - - `./scripts/generateExamples --onlyThisOne geoIndexSelect` will only produce one example - *geoIndexSelect* - - `./scripts/generateExamples --onlyThisOne 'MOD.*'` will only produce the examples matching that regex; Note that + - `./utils/generateExamples.sh --onlyThisOne geoIndexSelect` will only produce one example - *geoIndexSelect* + - `./utils/generateExamples.sh --onlyThisOne 'MOD.*'` will only produce the examples matching that regex; Note that examples with enumerations in their name may base on others in their series - so you should generate the whole group. - - `./scripts/generateExamples --server.endpoint tcp://127.0.0.1:8529` will utilize an existing arangod instead of starting a new one. + - `./utils/generateExamples.sh --server.endpoint tcp://127.0.0.1:8529` will utilize an existing arangod instead of starting a new one. this does seriously cut down the execution time. - - alternatively you can use generateExamples (i.e. on windows since the make target is not portable) like that: - `./scripts/generateExamples - --server.endpoint 'tcp://127.0.0.1:8529' - --withPython 3rdParty/V8-4.3.61/third_party/python_26/python26.exe + - you can use generateExamples like that: + `./utils/generateExamples.sh \ + --server.endpoint 'tcp://127.0.0.1:8529' \ + --withPython C:/tools/python2/python.exe \ --onlyThisOne 'MOD.*'` - `./Documentation/Scripts/allExamples.sh` generates a file where you can inspect all examples for readability. - - `make swagger` - on top level to generate the documentation interactively with the server; you may use + - `./utils/generateSwagger.sh` - on top level to generate the documentation interactively with the server; you may use [the swagger editor](https://github.com/swagger-api/swagger-editor) to revalidate whether *js/apps/system/_admin/aardvark/APP/api-docs.json* is accurate. - `cd Documentation/Books; make` - to generate the HTML documentation @@ -480,18 +443,17 @@ Read / use the documentation arangod Example tool ==================== -`make example` picks examples from the source code documentation, executes them, and creates a transcript including their results. -*Hint: Windows users may use ./scripts/generateExamples for this purpose* +`./utils/generateExamples.sh` picks examples from the code documentation, executes them, and creates a transcript including their results. Here is how its details work: - - all ending with *.cpp*, *.js* and *.mdpp* are searched. + - all *Documentation/DocuBlocks/*.md* and *Documentation/Books/*.mdpp* are searched. - all lines inside of source code starting with '///' are matched, all lines in .mdpp files. - an example start is marked with *@EXAMPLE_ARANGOSH_OUTPUT* or *@EXAMPLE_ARANGOSH_RUN* - the example is named by the string provided in brackets after the above key - the output is written to `Documentation/Examples/.generated` - examples end with *@END_EXAMPLE_[OUTPUT|RUN]* - - all code in between is executed as javascript in the **arangosh** while talking to a valid **arangod**. You may inspect the - generated js code in `/tmp/arangosh.examples.js` + - all code in between is executed as javascript in the **arangosh** while talking to a valid **arangod**. + You may inspect the generated js code in `/tmp/arangosh.examples.js` OUTPUT and RUN specifics --------------------------- @@ -535,7 +497,7 @@ sortable naming scheme so they're executed in sequence. Using `_ Date: Thu, 7 Apr 2016 12:59:38 +0200 Subject: [PATCH 02/20] stringify SSL opts --- .../HttpServer/ApplicationEndpointServer.cpp | 243 +++++++++++++++++- .../HttpServer/ApplicationEndpointServer.h | 1 + arangod/VocBase/vocbase.cpp | 1 + 3 files changed, 244 insertions(+), 1 deletion(-) diff --git a/arangod/HttpServer/ApplicationEndpointServer.cpp b/arangod/HttpServer/ApplicationEndpointServer.cpp index 042d9ef03b..dee698822d 100644 --- a/arangod/HttpServer/ApplicationEndpointServer.cpp +++ b/arangod/HttpServer/ApplicationEndpointServer.cpp @@ -360,7 +360,8 @@ bool ApplicationEndpointServer::createSslContext() { // set options SSL_CTX_set_options(_sslContext, (long)_sslOptions); - LOG(INFO) << "using SSL options: " << _sslOptions; + std::string sslOptions = stringifySslOptions(_sslOptions); + LOG(INFO) << "using SSL options: " << sslOptions; if (!_sslCipherList.empty()) { if (SSL_CTX_set_cipher_list(_sslContext, _sslCipherList.c_str()) != 1) { @@ -455,3 +456,243 @@ bool ApplicationEndpointServer::createSslContext() { return true; } + +std::string ApplicationEndpointServer::stringifySslOptions(uint64_t opts) const { + std::string result; + +#ifdef SSL_OP_MICROSOFT_SESS_ID_BUG + if (opts & SSL_OP_MICROSOFT_SESS_ID_BUG) { + result.append(", SSL_OP_MICROSOFT_SESS_ID_BUG"); + } +#endif + +#ifdef SSL_OP_NETSCAPE_CHALLENGE_BUG + if (opts & SSL_OP_NETSCAPE_CHALLENGE_BUG) { + result.append(", SSL_OP_NETSCAPE_CHALLENGE_BUG"); + } +#endif + +#ifdef SSL_OP_LEGACY_SERVER_CONNECT + if (opts & SSL_OP_LEGACY_SERVER_CONNECT) { + result.append(", SSL_OP_LEGACY_SERVER_CONNECT"); + } +#endif + +#ifdef SSL_OP_NETSCAPE_REUSE_CIPHER_CHANGE_BUG + if (opts & SSL_OP_NETSCAPE_REUSE_CIPHER_CHANGE_BUG) { + result.append(", SSL_OP_NETSCAPE_REUSE_CIPHER_CHANGE_BUG"); + } +#endif + +#ifdef SSL_OP_TLSEXT_PADDING + if (opts & SSL_OP_TLSEXT_PADDING) { + result.append(", SSL_OP_TLSEXT_PADDING"); + } +#endif + +#ifdef SSL_OP_MICROSOFT_BIG_SSLV3_BUFFER + if (opts & SSL_OP_MICROSOFT_BIG_SSLV3_BUFFER) { + result.append(", SSL_OP_MICROSOFT_BIG_SSLV3_BUFFER"); + } +#endif + +#ifdef SSL_OP_SAFARI_ECDHE_ECDSA_BUG + if (opts & SSL_OP_SAFARI_ECDHE_ECDSA_BUG) { + result.append(", SSL_OP_SAFARI_ECDHE_ECDSA_BUG"); + } +#endif + +#ifdef SSL_OP_SSLEAY_080_CLIENT_DH_BUG + if (opts & SSL_OP_SSLEAY_080_CLIENT_DH_BUG) { + result.append(", SSL_OP_SSLEAY_080_CLIENT_DH_BUG"); + } +#endif + +#ifdef SSL_OP_TLS_D5_BUG + if (opts & SSL_OP_TLS_D5_BUG) { + result.append(", SSL_OP_TLS_D5_BUG"); + } +#endif + +#ifdef SSL_OP_TLS_BLOCK_PADDING_BUG + if (opts & SSL_OP_TLS_BLOCK_PADDING_BUG) { + result.append(", SSL_OP_TLS_BLOCK_PADDING_BUG"); + } +#endif + +#ifdef SSL_OP_MSIE_SSLV2_RSA_PADDING + if (opts & SSL_OP_MSIE_SSLV2_RSA_PADDING) { + result.append(", SSL_OP_MSIE_SSLV2_RSA_PADDING"); + } +#endif + +#ifdef SSL_OP_SSLREF2_REUSE_CERT_TYPE_BUG + if (opts & SSL_OP_SSLREF2_REUSE_CERT_TYPE_BUG) { + result.append(", SSL_OP_SSLREF2_REUSE_CERT_TYPE_BUG"); + } +#endif + +#ifdef SSL_OP_DONT_INSERT_EMPTY_FRAGMENTS + if (opts & SSL_OP_DONT_INSERT_EMPTY_FRAGMENTS) { + result.append(", SSL_OP_DONT_INSERT_EMPTY_FRAGMENTS"); + } +#endif + +#ifdef SSL_OP_NO_QUERY_MTU + if (opts & SSL_OP_NO_QUERY_MTU) { + result.append(", SSL_OP_NO_QUERY_MTU"); + } +#endif + +#ifdef SSL_OP_COOKIE_EXCHANGE + if (opts & SSL_OP_COOKIE_EXCHANGE) { + result.append(", SSL_OP_COOKIE_EXCHANGE"); + } +#endif + +#ifdef SSL_OP_NO_TICKET + if (opts & SSL_OP_NO_TICKET) { + result.append(", SSL_OP_NO_TICKET"); + } +#endif + +#ifdef SSL_OP_CISCO_ANYCONNECT + if (opts & SSL_OP_CISCO_ANYCONNECT) { + result.append(", SSL_OP_CISCO_ANYCONNECT"); + } +#endif + +#ifdef SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION + if (opts & SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION) { + result.append(", SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION"); + } +#endif + +#ifdef SSL_OP_NO_COMPRESSION + if (opts & SSL_OP_NO_COMPRESSION) { + result.append(", SSL_OP_NO_COMPRESSION"); + } +#endif + +#ifdef SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION + if (opts & SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION) { + result.append(", SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION"); + } +#endif + +#ifdef SSL_OP_SINGLE_ECDH_USE + if (opts & SSL_OP_SINGLE_ECDH_USE) { + result.append(", SSL_OP_SINGLE_ECDH_USE"); + } +#endif + +#ifdef SSL_OP_SINGLE_DH_USE + if (opts & SSL_OP_SINGLE_DH_USE) { + result.append(", SSL_OP_SINGLE_DH_USE"); + } +#endif + +#ifdef SSL_OP_EPHEMERAL_RSA + if (opts & SSL_OP_EPHEMERAL_RSA) { + result.append(", SSL_OP_EPHEMERAL_RSA"); + } +#endif + +#ifdef SSL_OP_CIPHER_SERVER_PREFERENCE + if (opts & SSL_OP_CIPHER_SERVER_PREFERENCE) { + result.append(", SSL_OP_CIPHER_SERVER_PREFERENCE"); + } +#endif + +#ifdef SSL_OP_TLS_ROLLBACK_BUG + if (opts & SSL_OP_TLS_ROLLBACK_BUG) { + result.append(", SSL_OP_TLS_ROLLBACK_BUG"); + } +#endif + +#ifdef SSL_OP_NO_SSLv2 + if (opts & SSL_OP_NO_SSLv2) { + result.append(", SSL_OP_NO_SSLv2"); + } +#endif + +#ifdef SSL_OP_NO_SSLv3 + if (opts & SSL_OP_NO_SSLv3) { + result.append(", SSL_OP_NO_SSLv3"); + } +#endif + +#ifdef SSL_OP_NO_TLSv1 + if (opts & SSL_OP_NO_TLSv1) { + result.append(", SSL_OP_NO_TLSv1"); + } +#endif + +#ifdef SSL_OP_NO_TLSv1_2 + if (opts & SSL_OP_NO_TLSv1_2) { + result.append(", SSL_OP_NO_TLSv1_2"); + } +#endif + +#ifdef SSL_OP_NO_TLSv1_1 + if (opts & SSL_OP_NO_TLSv1_1) { + result.append(", SSL_OP_NO_TLSv1_1"); + } +#endif + +#ifdef SSL_OP_NO_DTLSv1 + if (opts & SSL_OP_NO_DTLSv1) { + result.append(", SSL_OP_NO_DTLSv1"); + } +#endif + +#ifdef SSL_OP_NO_DTLSv1_2 + if (opts & SSL_OP_NO_DTLSv1_2) { + result.append(", SSL_OP_NO_DTLSv1_2"); + } +#endif + +#ifdef SSL_OP_NO_SSL_MASK + if (opts & SSL_OP_NO_SSL_MASK) { + result.append(", SSL_OP_NO_SSL_MASK"); + } +#endif + +#ifdef SSL_OP_PKCS1_CHECK_1 + if (opts & SSL_OP_PKCS1_CHECK_1) { + result.append(", SSL_OP_PKCS1_CHECK_1"); + } +#endif + +#ifdef SSL_OP_PKCS1_CHECK_2 + if (opts & SSL_OP_PKCS1_CHECK_2) { + result.append(", SSL_OP_PKCS1_CHECK_2"); + } +#endif + +#ifdef SSL_OP_NETSCAPE_CA_DN_BUG + if (opts & SSL_OP_NETSCAPE_CA_DN_BUG) { + result.append(", SSL_OP_NETSCAPE_CA_DN_BUG"); + } +#endif + +#ifdef SSL_OP_NETSCAPE_DEMO_CIPHER_CHANGE_BUG + if (opts & SSL_OP_NETSCAPE_DEMO_CIPHER_CHANGE_BUG) { + result.append(", SSL_OP_NETSCAPE_DEMO_CIPHER_CHANGE_BUG"); + } +#endif + +#ifdef SSL_OP_CRYPTOPRO_TLSEXT_BUG + if (opts & SSL_OP_CRYPTOPRO_TLSEXT_BUG) { + result.append(", SSL_OP_CRYPTOPRO_TLSEXT_BUG"); + } +#endif + + if (result.empty()) { + return result; + } + + // strip initial comma + return result.substr(2); +} + diff --git a/arangod/HttpServer/ApplicationEndpointServer.h b/arangod/HttpServer/ApplicationEndpointServer.h index e30511645b..7a07e33397 100644 --- a/arangod/HttpServer/ApplicationEndpointServer.h +++ b/arangod/HttpServer/ApplicationEndpointServer.h @@ -94,6 +94,7 @@ class ApplicationEndpointServer : public ApplicationFeature { private: bool createSslContext(); + std::string stringifySslOptions(uint64_t opts) const; protected: ////////////////////////////////////////////////////////////////////////////// diff --git a/arangod/VocBase/vocbase.cpp b/arangod/VocBase/vocbase.cpp index 6215ef45c0..4c3151b2b6 100644 --- a/arangod/VocBase/vocbase.cpp +++ b/arangod/VocBase/vocbase.cpp @@ -1666,6 +1666,7 @@ TRI_vocbase_col_t* TRI_CreateCollectionVocBase( VPackBuilder builder; { VPackObjectBuilder b(&builder); + // note: cid may be modified by this function call collection = CreateCollection(vocbase, parameters, cid, writeMarker, builder); } From 513dc26fc17c7087263bd9582abeefcf2f453c15 Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Thu, 7 Apr 2016 13:00:30 +0200 Subject: [PATCH 03/20] make replication tests more deterministic --- .../RestHandler/RestReplicationHandler.cpp | 25 +- arangod/V8Server/v8-replication.cpp | 5 +- arangod/Wal/LogfileManager.cpp | 38 +- arangod/Wal/LogfileManager.h | 11 +- arangod/Wal/Slots.cpp | 9 +- arangod/Wal/Slots.h | 2 +- arangod/Wal/SynchronizerThread.cpp | 6 +- js/common/tests/replication/replication.js | 444 ++++++------------ 8 files changed, 224 insertions(+), 316 deletions(-) diff --git a/arangod/RestHandler/RestReplicationHandler.cpp b/arangod/RestHandler/RestReplicationHandler.cpp index 6a64703906..b90d9e9dcf 100644 --- a/arangod/RestHandler/RestReplicationHandler.cpp +++ b/arangod/RestHandler/RestReplicationHandler.cpp @@ -402,14 +402,14 @@ void RestReplicationHandler::handleCommandLoggerState() { VPackBuilder builder; builder.add(VPackValue(VPackValueType::Object)); // Base - arangodb::wal::LogfileManagerState const&& s = + arangodb::wal::LogfileManagerState const s = arangodb::wal::LogfileManager::instance()->state(); - std::string const lastTickString(StringUtils::itoa(s.lastTick)); // "state" part builder.add("state", VPackValue(VPackValueType::Object)); builder.add("running", VPackValue(true)); - builder.add("lastLogTick", VPackValue(lastTickString)); + builder.add("lastLogTick", VPackValue(std::to_string(s.lastCommittedTick))); + builder.add("lastUncommittedLogTick", VPackValue(std::to_string(s.lastAssignedTick))); builder.add("totalEvents", VPackValue(s.numEvents)); builder.add("time", VPackValue(s.timeString)); builder.close(); @@ -813,7 +813,7 @@ void RestReplicationHandler::handleTrampolineCoordinator() { void RestReplicationHandler::handleCommandLoggerFollow() { // determine start and end tick - arangodb::wal::LogfileManagerState state = + arangodb::wal::LogfileManagerState const state = arangodb::wal::LogfileManager::instance()->state(); TRI_voc_tick_t tickStart = 0; TRI_voc_tick_t tickEnd = UINT64_MAX; @@ -933,7 +933,7 @@ void RestReplicationHandler::handleCommandLoggerFollow() { if (res == TRI_ERROR_NO_ERROR) { bool const checkMore = (dump._lastFoundTick > 0 && - dump._lastFoundTick != state.lastDataTick); + dump._lastFoundTick != state.lastCommittedTick); // generate the result size_t const length = TRI_LengthStringBuffer(dump._buffer); @@ -954,7 +954,7 @@ void RestReplicationHandler::handleCommandLoggerFollow() { StringUtils::itoa(dump._lastFoundTick)); _response->setHeaderNC(TRI_REPLICATION_HEADER_LASTTICK, - StringUtils::itoa(state.lastTick)); + StringUtils::itoa(state.lastCommittedTick)); _response->setHeaderNC(TRI_REPLICATION_HEADER_ACTIVE, "true"); @@ -991,10 +991,10 @@ void RestReplicationHandler::handleCommandLoggerFollow() { void RestReplicationHandler::handleCommandDetermineOpenTransactions() { // determine start and end tick - arangodb::wal::LogfileManagerState state = + arangodb::wal::LogfileManagerState const state = arangodb::wal::LogfileManager::instance()->state(); TRI_voc_tick_t tickStart = 0; - TRI_voc_tick_t tickEnd = state.lastDataTick; + TRI_voc_tick_t tickEnd = state.lastCommittedTick; bool found; std::string const& value1 = _request->value("from", found); @@ -1100,12 +1100,12 @@ void RestReplicationHandler::handleCommandInventory() { // "state" builder.add("state", VPackValue(VPackValueType::Object)); - arangodb::wal::LogfileManagerState const&& s = + arangodb::wal::LogfileManagerState const s = arangodb::wal::LogfileManager::instance()->state(); builder.add("running", VPackValue(true)); - auto logTickString = std::to_string(s.lastTick); - builder.add("lastLogTick", VPackValue(logTickString)); + builder.add("lastLogTick", VPackValue(std::to_string(s.lastCommittedTick))); + builder.add("lastUncommittedLogTick", VPackValue(std::to_string(s.lastAssignedTick))); builder.add("totalEvents", VPackValue(s.numEvents)); builder.add("time", VPackValue(s.timeString)); @@ -3194,6 +3194,9 @@ void RestReplicationHandler::handleCommandSync() { config._password = password; config._includeSystem = includeSystem; config._verbose = verbose; + + // wait until all data in current logfile got synced + arangodb::wal::LogfileManager::instance()->waitForSync(5.0); InitialSyncer syncer(_vocbase, &config, restrictCollections, restrictType, verbose); diff --git a/arangod/V8Server/v8-replication.cpp b/arangod/V8Server/v8-replication.cpp index 2484156c24..a7039f62d9 100644 --- a/arangod/V8Server/v8-replication.cpp +++ b/arangod/V8Server/v8-replication.cpp @@ -51,14 +51,15 @@ static void JS_StateLoggerReplication( TRI_V8_TRY_CATCH_BEGIN(isolate); v8::HandleScope scope(isolate); - arangodb::wal::LogfileManagerState s = + arangodb::wal::LogfileManagerState const s = arangodb::wal::LogfileManager::instance()->state(); v8::Handle result = v8::Object::New(isolate); v8::Handle state = v8::Object::New(isolate); state->Set(TRI_V8_ASCII_STRING("running"), v8::True(isolate)); - state->Set(TRI_V8_ASCII_STRING("lastLogTick"), V8TickId(isolate, s.lastTick)); + state->Set(TRI_V8_ASCII_STRING("lastLogTick"), V8TickId(isolate, s.lastCommittedTick)); + state->Set(TRI_V8_ASCII_STRING("lastUncommittedLogTick"), V8TickId(isolate, s.lastAssignedTick)); state->Set(TRI_V8_ASCII_STRING("totalEvents"), v8::Number::New(isolate, (double)s.numEvents)); state->Set(TRI_V8_ASCII_STRING("time"), TRI_V8_STD_STRING(s.timeString)); diff --git a/arangod/Wal/LogfileManager.cpp b/arangod/Wal/LogfileManager.cpp index 9cde5a9d80..10fe639222 100644 --- a/arangod/Wal/LogfileManager.cpp +++ b/arangod/Wal/LogfileManager.cpp @@ -938,6 +938,42 @@ int LogfileManager::flush(bool waitForSync, bool waitForCollector, return res; } +//////////////////////////////////////////////////////////////////////////////// +/// wait until all changes to the current logfile are synced +//////////////////////////////////////////////////////////////////////////////// + +bool LogfileManager::waitForSync(double maxWait) { + TRI_ASSERT(!_inRecovery); + + double const end = TRI_microtime() + maxWait; + TRI_voc_tick_t lastAssignedTick = 0; + + while (true) { + // fill the state + LogfileManagerState state; + _slots->statistics(state.lastAssignedTick, state.lastCommittedTick, state.lastCommittedDataTick, state.numEvents); + + if (lastAssignedTick == 0) { + // get last assigned tick only once + lastAssignedTick = state.lastAssignedTick; + } + + // now compare last committed tick with first lastAssigned tick that we got + if (state.lastCommittedTick >= lastAssignedTick) { + // everything was already committed + return true; + } + + // not everything was committed yet. wait a bit + usleep(10000); + + if (TRI_microtime() >= end) { + // time's up! + return false; + } + } +} + //////////////////////////////////////////////////////////////////////////////// /// @brief re-inserts a logfile back into the inventory only //////////////////////////////////////////////////////////////////////////////// @@ -1656,7 +1692,7 @@ LogfileManagerState LogfileManager::state() { LogfileManagerState state; // now fill the state - _slots->statistics(state.lastTick, state.lastDataTick, state.numEvents); + _slots->statistics(state.lastAssignedTick, state.lastCommittedTick, state.lastCommittedDataTick, state.numEvents); state.timeString = getTimeString(); return state; diff --git a/arangod/Wal/LogfileManager.h b/arangod/Wal/LogfileManager.h index 87a139a360..8221e2cb54 100644 --- a/arangod/Wal/LogfileManager.h +++ b/arangod/Wal/LogfileManager.h @@ -66,8 +66,9 @@ struct LogfileRange { typedef std::vector LogfileRanges; struct LogfileManagerState { - TRI_voc_tick_t lastTick; - TRI_voc_tick_t lastDataTick; + TRI_voc_tick_t lastAssignedTick; + TRI_voc_tick_t lastCommittedTick; + TRI_voc_tick_t lastCommittedDataTick; uint64_t numEvents; std::string timeString; }; @@ -404,6 +405,12 @@ class LogfileManager : public rest::ApplicationFeature { ////////////////////////////////////////////////////////////////////////////// int flush(bool, bool, bool); + + ////////////////////////////////////////////////////////////////////////////// + /// wait until all changes to the current logfile are synced + ////////////////////////////////////////////////////////////////////////////// + + bool waitForSync(double); ////////////////////////////////////////////////////////////////////////////// /// @brief re-inserts a logfile back into the inventory only diff --git a/arangod/Wal/Slots.cpp b/arangod/Wal/Slots.cpp index 1049fc0204..03cbf21590 100644 --- a/arangod/Wal/Slots.cpp +++ b/arangod/Wal/Slots.cpp @@ -68,11 +68,14 @@ Slots::~Slots() { delete[] _slots; } /// @brief get the statistics of the slots //////////////////////////////////////////////////////////////////////////////// -void Slots::statistics(Slot::TickType& lastTick, Slot::TickType& lastDataTick, +void Slots::statistics(Slot::TickType& lastAssignedTick, + Slot::TickType& lastCommittedTick, + Slot::TickType& lastCommittedDataTick, uint64_t& numEvents) { MUTEX_LOCKER(mutexLocker, _lock); - lastTick = _lastCommittedTick; - lastDataTick = _lastCommittedDataTick; + lastAssignedTick = _lastAssignedTick; + lastCommittedTick = _lastCommittedTick; + lastCommittedDataTick = _lastCommittedDataTick; numEvents = _numEvents; } diff --git a/arangod/Wal/Slots.h b/arangod/Wal/Slots.h index 1d7fd6a470..91025e6703 100644 --- a/arangod/Wal/Slots.h +++ b/arangod/Wal/Slots.h @@ -95,7 +95,7 @@ class Slots { /// @brief get the statistics of the slots ////////////////////////////////////////////////////////////////////////////// - void statistics(Slot::TickType&, Slot::TickType&, uint64_t&); + void statistics(Slot::TickType&, Slot::TickType&, Slot::TickType&, uint64_t&); ////////////////////////////////////////////////////////////////////////////// /// @brief execute a flush operation diff --git a/arangod/Wal/SynchronizerThread.cpp b/arangod/Wal/SynchronizerThread.cpp index f075e4870e..0d493387ac 100644 --- a/arangod/Wal/SynchronizerThread.cpp +++ b/arangod/Wal/SynchronizerThread.cpp @@ -59,8 +59,10 @@ void SynchronizerThread::beginShutdown() { void SynchronizerThread::signalSync() { CONDITION_LOCKER(guard, _condition); - ++_waiting; - _condition.signal(); + if (++_waiting == 1) { + // only signal once + _condition.signal(); + } } //////////////////////////////////////////////////////////////////////////////// diff --git a/js/common/tests/replication/replication.js b/js/common/tests/replication/replication.js index 828377348f..fac20b2ac5 100644 --- a/js/common/tests/replication/replication.js +++ b/js/common/tests/replication/replication.js @@ -45,13 +45,20 @@ function ReplicationLoggerSuite () { var cn = "UnitTestsReplication"; var cn2 = "UnitTestsReplication2"; - var waitForSync = function () { - internal.wait(1, false); + var getLastLogTick = function () { + while (true) { + var s = replication.logger.state(); + + if (s.state.lastLogTick === s.state.lastUncommittedLogTick) { + return s.state.lastLogTick; + } + internal.wait(0.05, false); + } }; var getLogEntries = function (tick, type) { var result = [ ]; - waitForSync(); + getLastLogTick(); var exclude = function(name) { return (name.substr(0, 11) === '_statistics' || @@ -64,6 +71,10 @@ function ReplicationLoggerSuite () { var entries = REPLICATION_LOGGER_LAST(tick, "9999999999999999999"); + if (type === undefined) { + return entries; + } + if (Array.isArray(type)) { var found = false; entries.forEach(function(e) { @@ -140,6 +151,24 @@ function ReplicationLoggerSuite () { db._drop(cn2); }, +//////////////////////////////////////////////////////////////////////////////// +/// @brief get ticks +//////////////////////////////////////////////////////////////////////////////// + + testGetLoggerTicks : function () { + var state, tick; + + getLastLogTick(); + state = replication.logger.state().state; + assertTrue(state.running); + tick = state.lastLogTick; + assertTrue(typeof tick === 'string'); + assertMatch(/^\d+$/, state.lastLogTick); + tick = state.lastUncommittedLogTick; + assertTrue(typeof tick === 'string'); + assertMatch(/^\d+$/, state.lastLogTick); + }, + //////////////////////////////////////////////////////////////////////////////// /// @brief get state //////////////////////////////////////////////////////////////////////////////// @@ -147,13 +176,15 @@ function ReplicationLoggerSuite () { testGetLoggerState : function () { var state, tick, server; + getLastLogTick(); state = replication.logger.state().state; assertTrue(state.running); tick = state.lastLogTick; assertTrue(typeof tick === 'string'); assertNotEqual("", state.time); assertMatch(/^\d+-\d+-\d+T\d+:\d+:\d+Z$/, state.time); - + + // query the state again state = replication.logger.state().state; assertTrue(state.running); @@ -280,11 +311,7 @@ function ReplicationLoggerSuite () { //////////////////////////////////////////////////////////////////////////////// testLoggerCreateCollection : function () { - var state, tick; - - waitForSync(); - state = replication.logger.state().state; - tick = state.lastLogTick; + var tick = getLastLogTick(); var c = db._create(cn); var entry = getLogEntries(tick, 2000)[0]; @@ -302,13 +329,9 @@ function ReplicationLoggerSuite () { //////////////////////////////////////////////////////////////////////////////// testLoggerDropCollection : function () { - var state, tick; - var c = db._create(cn); - waitForSync(); - state = replication.logger.state().state; - tick = state.lastLogTick; + var tick = getLastLogTick(); db._drop(cn); var entry = getLogEntries(tick, 2001)[0]; @@ -322,13 +345,9 @@ function ReplicationLoggerSuite () { //////////////////////////////////////////////////////////////////////////////// testLoggerRenameCollection : function () { - var state, tick; - var c = db._create(cn); - waitForSync(); - state = replication.logger.state().state; - tick = state.lastLogTick; + var tick = getLastLogTick(); c.rename(cn2); var entry = getLogEntries(tick, 2002)[0]; @@ -343,19 +362,15 @@ function ReplicationLoggerSuite () { //////////////////////////////////////////////////////////////////////////////// testLoggerPropertiesCollection : function () { - var state, tick; - var c = db._create(cn); - waitForSync(); - state = replication.logger.state().state; - tick = state.lastLogTick; + var tick = getLastLogTick(); c.properties({ waitForSync: true, journalSize: 2097152 }); var entry = getLogEntries(tick, 2003)[0]; assertEqual(2003, entry.type); - assertEqual(c._id, entry.cid); + assertEqual(c._id, entry.cid, JSON.stringify(entry)); assertEqual(c._id, entry.data.cid); assertEqual(cn, entry.data.name); assertEqual(2, entry.data.type); @@ -369,25 +384,21 @@ function ReplicationLoggerSuite () { //////////////////////////////////////////////////////////////////////////////// testLoggerIncludedSystemCollection1 : function () { - var state, tick; - var c = db._collection("_graphs"); - state = replication.logger.state().state; - tick = state.lastLogTick; + var tick = getLastLogTick(); var doc = c.save({ "test": 1 }); var entry = getLogEntries(tick, 2300)[0]; assertEqual(2300, entry.type); - assertEqual(c._id, entry.cid); + assertEqual(c._id, entry.cid, JSON.stringify(entry)); - state = replication.logger.state().state; - tick = state.lastLogTick; + tick = getLastLogTick(); c.remove(doc._key); entry = getLogEntries(tick, 2302)[0]; assertEqual(2302, entry.type); - assertEqual(c._id, entry.cid); + assertEqual(c._id, entry.cid, JSON.stringify(entry)); assertEqual(doc._key, entry.data._key); }, @@ -396,25 +407,21 @@ function ReplicationLoggerSuite () { //////////////////////////////////////////////////////////////////////////////// testLoggerIncludedSystemCollection2 : function () { - var state, tick; - var c = db._collection("_users"); - state = replication.logger.state().state; - tick = state.lastLogTick; + var tick = getLastLogTick(); var doc = c.save({ "test": 1 }); var entry = getLogEntries(tick, 2300)[0]; assertEqual(2300, entry.type); - assertEqual(c._id, entry.cid); + assertEqual(c._id, entry.cid, JSON.stringify(entry)); - state = replication.logger.state().state; - tick = state.lastLogTick; + tick = getLastLogTick(); c.remove(doc._key); entry = getLogEntries(tick, 2302)[0]; assertEqual(2302, entry.type); - assertEqual(c._id, entry.cid); + assertEqual(c._id, entry.cid, JSON.stringify(entry)); assertEqual(doc._key, entry.data._key); }, @@ -423,47 +430,41 @@ function ReplicationLoggerSuite () { //////////////////////////////////////////////////////////////////////////////// testLoggerSystemCollection : function () { - var state, tick; - db._drop("_unitfoxx"); db._drop("_unittests"); - state = replication.logger.state().state; - tick = state.lastLogTick; + var tick = getLastLogTick(); var c = db._create("_unittests", { isSystem : true }); var entry = getLogEntries(tick, 2000)[0]; assertEqual(2000, entry.type); - assertEqual(c._id, entry.cid); + assertEqual(c._id, entry.cid, JSON.stringify(entry)); assertEqual(c.name(), entry.data.name); - state = replication.logger.state().state; - tick = state.lastLogTick; + tick = getLastLogTick(); c.properties({ waitForSync : true }); entry = getLogEntries(tick, 2003)[0]; assertEqual(2003, entry.type); - assertEqual(c._id, entry.cid); + assertEqual(c._id, entry.cid, JSON.stringify(entry)); assertEqual(true, entry.data.waitForSync); - state = replication.logger.state().state; - tick = state.lastLogTick; + tick = getLastLogTick(); c.rename("_unitfoxx"); entry = getLogEntries(tick, 2002)[0]; assertEqual(2002, entry.type); - assertEqual(c._id, entry.cid); + assertEqual(c._id, entry.cid, JSON.stringify(entry)); assertEqual("_unitfoxx", entry.data.name); - state = replication.logger.state().state; - tick = state.lastLogTick; + tick = getLastLogTick(); c.rename("_unittests"); entry = getLogEntries(tick, 2002)[0]; assertEqual(2002, entry.type); - assertEqual(c._id, entry.cid); + assertEqual(c._id, entry.cid, JSON.stringify(entry)); assertEqual("_unittests", entry.data.name); }, @@ -472,14 +473,10 @@ function ReplicationLoggerSuite () { //////////////////////////////////////////////////////////////////////////////// testLoggerTruncateCollection1 : function () { - var state, tick; - var c = db._create(cn); c.save({ "test": 1, "_key": "abc" }); - waitForSync(); - state = replication.logger.state().state; - tick = state.lastLogTick; + var tick = getLastLogTick(); c.truncate(); var entry = getLogEntries(tick, 2302); @@ -490,9 +487,7 @@ function ReplicationLoggerSuite () { c.save({ "test": 1, "_key": "abc" }); - waitForSync(); - state = replication.logger.state().state; - tick = state.lastLogTick; + tick = getLastLogTick(); c.truncate(); entry = getLogEntries(tick, 2302); @@ -506,18 +501,16 @@ function ReplicationLoggerSuite () { //////////////////////////////////////////////////////////////////////////////// testLoggerTruncateCollection2 : function () { - var state, tick, i; + var i; var c = db._create(cn); for (i = 0; i < 100; ++i) { c.save({ "test": 1, "_key": "test" + i }); } - waitForSync(); - state = replication.logger.state().state; - tick = state.lastLogTick; + var tick = getLastLogTick(); c.truncate(); - waitForSync(); + getLastLogTick(); var entry = getLogEntries(tick, [ 2200, 2201, 2202, 2302 ]); assertEqual(102, entry.length); @@ -546,19 +539,16 @@ function ReplicationLoggerSuite () { //////////////////////////////////////////////////////////////////////////////// testLoggerCreateIndexHash1 : function () { - var state, tick; - var c = db._create(cn); - state = replication.logger.state().state; - tick = state.lastLogTick; + var tick = getLastLogTick(); c.ensureUniqueConstraint("a", "b"); var idx = c.getIndexes()[1]; var entry = getLogEntries(tick, 2100)[0]; assertTrue(2100, entry.type); - assertEqual(c._id, entry.cid); + assertEqual(c._id, entry.cid, JSON.stringify(entry)); assertEqual(idx.id.replace(/^.*\//, ''), entry.data.id); assertEqual("hash", entry.data.type); assertEqual(true, entry.data.unique); @@ -571,18 +561,15 @@ function ReplicationLoggerSuite () { //////////////////////////////////////////////////////////////////////////////// testLoggerCreateIndexHash2 : function () { - var state, tick; - var c = db._create(cn); - state = replication.logger.state().state; - tick = state.lastLogTick; + var tick = getLastLogTick(); c.ensureHashIndex("a"); var idx = c.getIndexes()[1]; var entry = getLogEntries(tick, 2100)[0]; - assertEqual(c._id, entry.cid); + assertEqual(c._id, entry.cid, JSON.stringify(entry)); assertEqual(idx.id.replace(/^.*\//, ''), entry.data.id); assertEqual("hash", entry.data.type); assertEqual(false, entry.data.unique); @@ -595,19 +582,17 @@ function ReplicationLoggerSuite () { //////////////////////////////////////////////////////////////////////////////// testLoggerCreateIndexSparseHash1 : function () { - var state, tick; - var c = db._create(cn); - state = replication.logger.state().state; - tick = state.lastLogTick; + var tick = getLastLogTick(); c.ensureUniqueConstraint("a", "b", { sparse: true }); + var idx = c.getIndexes()[1]; var entry = getLogEntries(tick, 2100)[0]; assertTrue(2100, entry.type); - assertEqual(c._id, entry.cid); + assertEqual(c._id, entry.cid, JSON.stringify(entry)); assertEqual(idx.id.replace(/^.*\//, ''), entry.data.id); assertEqual("hash", entry.data.type); assertEqual(true, entry.data.unique); @@ -620,18 +605,16 @@ function ReplicationLoggerSuite () { //////////////////////////////////////////////////////////////////////////////// testLoggerCreateIndexSparseHash2 : function () { - var state, tick; - var c = db._create(cn); - state = replication.logger.state().state; - tick = state.lastLogTick; + var tick = getLastLogTick(); c.ensureHashIndex("a", { sparse: true }); + var idx = c.getIndexes()[1]; var entry = getLogEntries(tick, 2100)[0]; - assertEqual(c._id, entry.cid); + assertEqual(c._id, entry.cid, JSON.stringify(entry)); assertEqual(idx.id.replace(/^.*\//, ''), entry.data.id); assertEqual("hash", entry.data.type); assertEqual(false, entry.data.unique); @@ -644,19 +627,17 @@ function ReplicationLoggerSuite () { //////////////////////////////////////////////////////////////////////////////// testLoggerCreateIndexSkiplist1 : function () { - var state, tick; - var c = db._create(cn); - state = replication.logger.state().state; - tick = state.lastLogTick; + var tick = getLastLogTick(); c.ensureSkiplist("a", "b", "c"); + var idx = c.getIndexes()[1]; var entry = getLogEntries(tick, 2100)[0]; assertTrue(2100, entry.type); - assertEqual(c._id, entry.cid); + assertEqual(c._id, entry.cid, JSON.stringify(entry)); assertEqual(idx.id.replace(/^.*\//, ''), entry.data.id); assertEqual("skiplist", entry.data.type); assertEqual(false, entry.data.unique); @@ -669,19 +650,16 @@ function ReplicationLoggerSuite () { //////////////////////////////////////////////////////////////////////////////// testLoggerCreateIndexSkiplist2 : function () { - var state, tick; - var c = db._create(cn); - state = replication.logger.state().state; - tick = state.lastLogTick; + var tick = getLastLogTick(); c.ensureUniqueSkiplist("a"); + var idx = c.getIndexes()[1]; - var entry1 = getLogEntries(tick, 2100)[0]; - entry1 = true;/// TODO var entry = getLogEntries(tick, 2100)[0]; - assertEqual(c._id, entry.cid); + + assertEqual(c._id, entry.cid, JSON.stringify(entry)); assertEqual(idx.id.replace(/^.*\//, ''), entry.data.id); assertEqual("skiplist", entry.data.type); assertEqual(true, entry.data.unique); @@ -694,19 +672,17 @@ function ReplicationLoggerSuite () { //////////////////////////////////////////////////////////////////////////////// testLoggerCreateIndexSparseSkiplist1 : function () { - var state, tick; - var c = db._create(cn); - state = replication.logger.state().state; - tick = state.lastLogTick; + var tick = getLastLogTick(); c.ensureSkiplist("a", "b", "c", { sparse: true }); + var idx = c.getIndexes()[1]; var entry = getLogEntries(tick, 2100)[0]; assertTrue(2100, entry.type); - assertEqual(c._id, entry.cid); + assertEqual(c._id, entry.cid, JSON.stringify(entry)); assertEqual(idx.id.replace(/^.*\//, ''), entry.data.id); assertEqual("skiplist", entry.data.type); assertEqual(false, entry.data.unique); @@ -719,19 +695,16 @@ function ReplicationLoggerSuite () { //////////////////////////////////////////////////////////////////////////////// testLoggerCreateIndexSparseSkiplist2 : function () { - var state, tick; - var c = db._create(cn); - state = replication.logger.state().state; - tick = state.lastLogTick; + var tick = getLastLogTick(); c.ensureUniqueSkiplist("a", { sparse: true }); + var idx = c.getIndexes()[1]; - var entry1 = getLogEntries(tick, 2100)[0]; - entry1 = true;/// TODO var entry = getLogEntries(tick, 2100)[0]; - assertEqual(c._id, entry.cid); + + assertEqual(c._id, entry.cid, JSON.stringify(entry)); assertEqual(idx.id.replace(/^.*\//, ''), entry.data.id); assertEqual("skiplist", entry.data.type); assertEqual(true, entry.data.unique); @@ -744,18 +717,15 @@ function ReplicationLoggerSuite () { //////////////////////////////////////////////////////////////////////////////// testLoggerCreateIndexFulltext1 : function () { - var state, tick; - var c = db._create(cn); - state = replication.logger.state().state; - tick = state.lastLogTick; + var tick = getLastLogTick(); c.ensureFulltextIndex("a", 5); var idx = c.getIndexes()[1]; var entry = getLogEntries(tick, 2100)[0]; assertTrue(2100, entry.type); - assertEqual(c._id, entry.cid); + assertEqual(c._id, entry.cid, JSON.stringify(entry)); assertEqual(idx.id.replace(/^.*\//, ''), entry.data.id); assertEqual("fulltext", entry.data.type); assertEqual(false, entry.data.unique); @@ -768,21 +738,16 @@ function ReplicationLoggerSuite () { //////////////////////////////////////////////////////////////////////////////// testLoggerCreateIndexGeo1 : function () { - var state, tick; - var c = db._create(cn); - state = replication.logger.state().state; - tick = state.lastLogTick; - var count = getLogEntries(); - count = true; /// TODO + var tick = getLastLogTick(); c.ensureGeoIndex("a", "b"); var idx = c.getIndexes()[1]; var entry = getLogEntries(tick, 2100)[0]; assertTrue(2100, entry.type); - assertEqual(c._id, entry.cid); + assertEqual(c._id, entry.cid, JSON.stringify(entry)); assertEqual(idx.id.replace(/^.*\//, ''), entry.data.id); assertEqual("geo2", entry.data.type); assertEqual(false, entry.data.unique); @@ -795,19 +760,16 @@ function ReplicationLoggerSuite () { //////////////////////////////////////////////////////////////////////////////// testLoggerCreateIndexGeo2 : function () { - var state, tick; - var c = db._create(cn); - state = replication.logger.state().state; - tick = state.lastLogTick; + var tick = getLastLogTick(); c.ensureGeoIndex("a", true); var idx = c.getIndexes()[1]; var entry = getLogEntries(tick, 2100)[0]; assertTrue(2100, entry.type); - assertEqual(c._id, entry.cid); + assertEqual(c._id, entry.cid, JSON.stringify(entry)); assertEqual(idx.id.replace(/^.*\//, ''), entry.data.id); assertEqual("geo1", entry.data.type); assertEqual(false, entry.data.unique); @@ -820,19 +782,16 @@ function ReplicationLoggerSuite () { //////////////////////////////////////////////////////////////////////////////// testLoggerCreateIndexGeo3 : function () { - var state, tick; - var c = db._create(cn); - state = replication.logger.state().state; - tick = state.lastLogTick; + var tick = getLastLogTick(); c.ensureGeoConstraint("a", "b", true); var idx = c.getIndexes()[1]; var entry = getLogEntries(tick, 2100)[0]; assertTrue(2100, entry.type); - assertEqual(c._id, entry.cid); + assertEqual(c._id, entry.cid, JSON.stringify(entry)); assertEqual(idx.id.replace(/^.*\//, ''), entry.data.id); assertEqual("geo2", entry.data.type); assertEqual(false, entry.data.unique); @@ -847,19 +806,16 @@ function ReplicationLoggerSuite () { //////////////////////////////////////////////////////////////////////////////// testLoggerCreateIndexGeo4 : function () { - var state, tick; - var c = db._create(cn); - state = replication.logger.state().state; - tick = state.lastLogTick; + var tick = getLastLogTick(); c.ensureGeoConstraint("a", "b", false); var idx = c.getIndexes()[1]; var entry = getLogEntries(tick, 2100)[0]; assertTrue(2100, entry.type); - assertEqual(c._id, entry.cid); + assertEqual(c._id, entry.cid, JSON.stringify(entry)); assertEqual(idx.id.replace(/^.*\//, ''), entry.data.id); assertEqual("geo2", entry.data.type); assertEqual(false, entry.data.unique); @@ -874,19 +830,16 @@ function ReplicationLoggerSuite () { //////////////////////////////////////////////////////////////////////////////// testLoggerCreateIndexGeo5 : function () { - var state, tick; - var c = db._create(cn); - state = replication.logger.state().state; - tick = state.lastLogTick; + var tick = getLastLogTick(); c.ensureGeoConstraint("a", true); var idx = c.getIndexes()[1]; var entry = getLogEntries(tick, 2100)[0]; assertTrue(2100, entry.type); - assertEqual(c._id, entry.cid); + assertEqual(c._id, entry.cid, JSON.stringify(entry)); assertEqual(idx.id.replace(/^.*\//, ''), entry.data.id); assertEqual("geo1", entry.data.type); assertEqual(false, entry.data.unique); @@ -901,13 +854,10 @@ function ReplicationLoggerSuite () { //////////////////////////////////////////////////////////////////////////////// testLoggerDropIndex : function () { - var state, tick; - var c = db._create(cn); c.ensureUniqueConstraint("a", "b"); - - state = replication.logger.state().state; - tick = state.lastLogTick; + + var tick = getLastLogTick(); // use index at #1 (#0 is primary index) var idx = c.getIndexes()[1]; @@ -915,7 +865,7 @@ function ReplicationLoggerSuite () { var entry = getLogEntries(tick, 2101)[0]; assertTrue(2101, entry.type); - assertEqual(c._id, entry.cid); + assertEqual(c._id, entry.cid, JSON.stringify(entry)); assertEqual(idx.id.replace(/^.*\//, ''), entry.data.id); }, @@ -924,32 +874,28 @@ function ReplicationLoggerSuite () { //////////////////////////////////////////////////////////////////////////////// testLoggerSaveDocument : function () { - var state, tick; - var c = db._create(cn); - - state = replication.logger.state().state; - tick = state.lastLogTick; + + var tick = getLastLogTick(); c.save({ "test": 1, "_key": "abc" }); var rev = c.document("abc")._rev; var entry = getLogEntries(tick, 2300)[0]; assertEqual(2300, entry.type); - assertEqual(c._id, entry.cid); + assertEqual(c._id, entry.cid, JSON.stringify(entry)); assertEqual("abc", entry.data._key); assertEqual(rev, entry.data._rev); assertEqual(1, entry.data.test); - state = replication.logger.state().state; - tick = state.lastLogTick; + tick = getLastLogTick(); c.save({ "test": 2, "foo" : "bar", "_key": "12345" }); rev = c.document("12345")._rev; entry = getLogEntries(tick, 2300)[0]; assertEqual(2300, entry.type); - assertEqual(c._id, entry.cid); + assertEqual(c._id, entry.cid, JSON.stringify(entry)); assertEqual("12345", entry.data._key); assertEqual(rev, entry.data._rev); assertEqual(2, entry.data.test); @@ -971,12 +917,9 @@ function ReplicationLoggerSuite () { //////////////////////////////////////////////////////////////////////////////// testLoggerSaveDocuments : function () { - var state, tick; - var c = db._create(cn); - - state = replication.logger.state().state; - tick = state.lastLogTick; + + var tick = getLastLogTick(); for (var i = 0; i < 100; ++i) { c.save({ "test": 1, "_key": "test" + i }); @@ -991,33 +934,28 @@ function ReplicationLoggerSuite () { //////////////////////////////////////////////////////////////////////////////// testLoggerDeleteDocument : function () { - var state, tick; - var c = db._create(cn); c.save({ "test": 1, "_key": "abc" }); c.save({ "test": 1, "_key": "12345" }); - state = replication.logger.state().state; - tick = state.lastLogTick; + var tick = getLastLogTick(); c.remove("abc"); var entry = getLogEntries(tick, 2302)[0]; assertEqual(2302, entry.type); - assertEqual(c._id, entry.cid); + assertEqual(c._id, entry.cid, JSON.stringify(entry)); assertEqual("abc", entry.data._key); - state = replication.logger.state().state; - tick = state.lastLogTick; + tick = getLastLogTick(); c.remove("12345"); entry = getLogEntries(tick, 2302)[0]; assertEqual(2302, entry.type); - assertEqual(c._id, entry.cid); + assertEqual(c._id, entry.cid, JSON.stringify(entry)); assertEqual("12345", entry.data._key); - state = replication.logger.state().state; - tick = state.lastLogTick; + tick = getLastLogTick(); try { c.remove("12345"); @@ -1035,15 +973,11 @@ function ReplicationLoggerSuite () { //////////////////////////////////////////////////////////////////////////////// testLoggerUpdateDocument : function () { - var state, tick; - var c = db._create(cn); + var tick = getLastLogTick(); c.save({ "test": 2, "_key": "abc" }); c.save({ "test": 1, "_key": "12345" }); - state = replication.logger.state().state; - tick = state.lastLogTick; - c.update("abc", { "test" : 2 }); var entry = getLogEntries(tick, 2300); @@ -1058,18 +992,16 @@ function ReplicationLoggerSuite () { assertEqual(1, entry[1].data.test); - state = replication.logger.state().state; - tick = state.lastLogTick; + tick = getLastLogTick(); c.update("abc", { "test" : 3 }); entry = getLogEntries(tick, 2300)[0]; assertEqual(2300, entry.type); - assertEqual(c._id, entry.cid); + assertEqual(c._id, entry.cid, JSON.stringify(entry)); assertEqual("abc", entry.data._key); assertEqual(3, entry.data.test); - state = replication.logger.state().state; - tick = state.lastLogTick; + tick = getLastLogTick(); c.update("abc", { "test" : 3 }); c.update("12345", { "test" : 2 }); @@ -1077,9 +1009,7 @@ function ReplicationLoggerSuite () { entry = getLogEntries(tick, 2300); assertEqual(3, entry.length); - waitForSync(); - state = replication.logger.state().state; - tick = state.lastLogTick; + tick = getLastLogTick(); try { c.update("thefoxx", { }); @@ -1097,15 +1027,11 @@ function ReplicationLoggerSuite () { //////////////////////////////////////////////////////////////////////////////// testLoggerReplaceDocument : function () { - var state, tick; - var c = db._create(cn); + var tick = getLastLogTick(); c.save({ "test": 2, "_key": "abc" }); c.save({ "test": 1, "_key": "12345" }); - state = replication.logger.state().state; - tick = state.lastLogTick; - c.replace("abc", { "test" : 2 }); var entry = getLogEntries(tick, 2300); assertEqual(2300, entry[0].type); @@ -1118,8 +1044,7 @@ function ReplicationLoggerSuite () { assertEqual("12345", entry[1].data._key); assertEqual(1, entry[1].data.test); - state = replication.logger.state().state; - tick = state.lastLogTick; + tick = getLastLogTick(); c.replace("abc", { "test" : 3 }); c.replace("abc", { "test" : 3 }); c.replace("12345", { "test" : 2 }); @@ -1128,8 +1053,7 @@ function ReplicationLoggerSuite () { entry = getLogEntries(tick, 2300); assertEqual(4, entry.length); - state = replication.logger.state().state; - tick = state.lastLogTick; + tick = getLastLogTick(); try { c.replace("thefoxx", { }); @@ -1147,13 +1071,10 @@ function ReplicationLoggerSuite () { //////////////////////////////////////////////////////////////////////////////// testLoggerSaveEdge : function () { - var state, tick; - var c = db._create(cn); var e = db._createEdgeCollection(cn2); - state = replication.logger.state().state; - tick = state.lastLogTick; + var tick = getLastLogTick(); e.save(cn + "/test1", cn + "/test2", { "test": 1, "_key": "abc" }); @@ -1165,8 +1086,7 @@ function ReplicationLoggerSuite () { assertEqual(c.name() + "/test2", entry.data._to); assertEqual(1, entry.data.test); - state = replication.logger.state().state; - tick = state.lastLogTick; + tick = getLastLogTick(); e.save(cn + "/test3", cn + "/test4", { "test": [ 99, false ], "_key": "12345" }); entry = getLogEntries(tick, 2300)[0]; @@ -1177,8 +1097,7 @@ function ReplicationLoggerSuite () { assertEqual(c.name() + "/test4", entry.data._to); assertEqual([ 99, false ], entry.data.test); - state = replication.logger.state().state; - tick = state.lastLogTick; + tick = getLastLogTick(); try { e.save(); @@ -1196,16 +1115,13 @@ function ReplicationLoggerSuite () { //////////////////////////////////////////////////////////////////////////////// testLoggerDeleteEdge : function () { - var state, tick; - db._create(cn); var e = db._createEdgeCollection(cn2); e.save(cn + "/test1", cn + "/test2", { "test": 1, "_key": "abc" }); e.save(cn + "/test3", cn + "/test4", { "test": 1, "_key": "12345" }); - state = replication.logger.state().state; - tick = state.lastLogTick; + var tick = getLastLogTick(); e.remove("abc"); @@ -1216,9 +1132,7 @@ function ReplicationLoggerSuite () { e.remove("12345"); - waitForSync(); - state = replication.logger.state().state; - tick = state.lastLogTick; + tick = getLastLogTick(); try { e.remove("12345"); @@ -1236,16 +1150,12 @@ function ReplicationLoggerSuite () { //////////////////////////////////////////////////////////////////////////////// testLoggerUpdateEdge : function () { - var state, tick; - var c = db._create(cn); var e = db._createEdgeCollection(cn2); + var tick = getLastLogTick(); e.save(cn + "/test1", cn + "/test2", { "test": 2, "_key": "abc" }); e.save(cn + "/test3", cn + "/test4", { "test": 1, "_key": "12345" }); - state = replication.logger.state().state; - tick = state.lastLogTick; - e.update("abc", { "test" : 2 }); var entry = getLogEntries(tick, 2300); assertEqual(2300, entry[0].type); @@ -1262,8 +1172,7 @@ function ReplicationLoggerSuite () { assertEqual(c.name() + "/test3", entry[1].data._from); assertEqual(c.name() + "/test4", entry[1].data._to); - state = replication.logger.state().state; - tick = state.lastLogTick; + tick = getLastLogTick(); e.update("abc", { "test" : 3 }); e.update("12345", { "test" : 2 }); @@ -1272,8 +1181,7 @@ function ReplicationLoggerSuite () { assertEqual(3, entry.length); - state = replication.logger.state().state; - tick = state.lastLogTick; + tick = getLastLogTick(); try { e.update("thefoxx", { }); @@ -1291,16 +1199,13 @@ function ReplicationLoggerSuite () { //////////////////////////////////////////////////////////////////////////////// testLoggerReplaceEdge : function () { - var state, tick; - var c = db._create(cn); var e = db._createEdgeCollection(cn2); + var tick = getLastLogTick(); + e.save(cn + "/test1", cn + "/test2", { "test": 2, "_key": "abc" }); e.save(cn + "/test3", cn + "/test4", { "test": 1, "_key": "12345" }); - state = replication.logger.state().state; - tick = state.lastLogTick; - e.replace("abc", { _from: c.name() + "/test1", _to: c.name() + "/test2", "test" : 2 }); var entry = getLogEntries(tick, 2300); assertEqual(2300, entry[0].type); @@ -1317,8 +1222,7 @@ function ReplicationLoggerSuite () { assertEqual(c.name() + "/test3", entry[1].data._from); assertEqual(c.name() + "/test4", entry[1].data._to); - state = replication.logger.state().state; - tick = state.lastLogTick; + tick = getLastLogTick(); e.replace("abc", { _from: cn + "/test1", _to: cn + "/test2", "test" : 3 }); e.replace("abc", { _from: cn + "/test1", _to: cn + "/test2", "test" : 3 }); @@ -1328,8 +1232,7 @@ function ReplicationLoggerSuite () { entry = getLogEntries(tick, 2300); assertEqual(4, entry.length); - state = replication.logger.state().state; - tick = state.lastLogTick; + tick = getLastLogTick(); try { e.replace("thefoxx", { }); @@ -1347,12 +1250,9 @@ function ReplicationLoggerSuite () { //////////////////////////////////////////////////////////////////////////////// testLoggerTransactionEmpty : function () { - var state, tick; - db._create(cn); - state = replication.logger.state().state; - tick = state.lastLogTick; + var tick = getLastLogTick(); var actual = db._executeTransaction({ collections: { @@ -1372,13 +1272,10 @@ function ReplicationLoggerSuite () { //////////////////////////////////////////////////////////////////////////////// testLoggerTransactionRead1 : function () { - var state, tick; - var c = db._create(cn); c.save({ "test" : 1 }); - state = replication.logger.state().state; - tick = state.lastLogTick; + var tick = getLastLogTick(); var actual = db._executeTransaction({ collections: { @@ -1399,13 +1296,10 @@ function ReplicationLoggerSuite () { //////////////////////////////////////////////////////////////////////////////// testLoggerTransactionRead2 : function () { - var state, tick; - var c = db._create(cn); c.save({ "test" : 1, "_key": "abc" }); - state = replication.logger.state().state; - tick = state.lastLogTick; + var tick = getLastLogTick(); var actual = db._executeTransaction({ collections: { @@ -1433,12 +1327,9 @@ function ReplicationLoggerSuite () { //////////////////////////////////////////////////////////////////////////////// testLoggerTransactionRead3 : function () { - var state, tick; - db._create(cn); - state = replication.logger.state().state; - tick = state.lastLogTick; + var tick = getLastLogTick(); try { var actual = db._executeTransaction({ @@ -1471,12 +1362,8 @@ function ReplicationLoggerSuite () { //////////////////////////////////////////////////////////////////////////////// testLoggerTransactionWrite1 : function () { - var state, tick; - var c = db._create(cn); - c = true;// todo - state = replication.logger.state().state; - tick = state.lastLogTick; + var tick = getLastLogTick(); var actual = db._executeTransaction({ collections: { @@ -1497,12 +1384,9 @@ function ReplicationLoggerSuite () { //////////////////////////////////////////////////////////////////////////////// testLoggerTransactionWrite2 : function () { - var state, tick; - var c = db._create(cn); - state = replication.logger.state().state; - tick = state.lastLogTick; + var tick = getLastLogTick(); var actual = db._executeTransaction({ collections: { @@ -1539,12 +1423,9 @@ function ReplicationLoggerSuite () { //////////////////////////////////////////////////////////////////////////////// testLoggerTransactionWrite3 : function () { - var state, tick; - db._create(cn); - state = replication.logger.state().state; - tick = state.lastLogTick; + var tick = getLastLogTick(); try { db._executeTransaction({ @@ -1582,13 +1463,10 @@ function ReplicationLoggerSuite () { //////////////////////////////////////////////////////////////////////////////// testLoggerTransactionWrite4 : function () { - var state, tick; - db._create(cn); db._create(cn2); - state = replication.logger.state().state; - tick = state.lastLogTick; + var tick = getLastLogTick(); try { db._executeTransaction({ @@ -1619,13 +1497,10 @@ function ReplicationLoggerSuite () { //////////////////////////////////////////////////////////////////////////////// testLoggerTransactionWrite5 : function () { - var state, tick; - db._create(cn); db._create(cn2); - state = replication.logger.state().state; - tick = state.lastLogTick; + var tick = getLastLogTick(); db._executeTransaction({ collections: { @@ -1660,13 +1535,10 @@ function ReplicationLoggerSuite () { //////////////////////////////////////////////////////////////////////////////// testLoggerTransactionWrite6 : function () { - var state, tick; - var c1 = db._create(cn); var c2 = db._create(cn2); - state = replication.logger.state().state; - tick = state.lastLogTick; + var tick = getLastLogTick(); db._executeTransaction({ collections: { @@ -1705,11 +1577,9 @@ function ReplicationLoggerSuite () { //////////////////////////////////////////////////////////////////////////////// testLoggerTransactionExcluded : function () { - var state, tick; var c = db._create(cn); - state = replication.logger.state().state; - tick = state.lastLogTick; + var tick = getLastLogTick(); db._executeTransaction({ collections: { @@ -1776,9 +1646,7 @@ function ReplicationApplierSuite () { //////////////////////////////////////////////////////////////////////////////// testStartApplierNoConfig : function () { - var state; - - state = replication.applier.state(); + var state = replication.applier.state(); assertFalse(state.state.running); @@ -1797,9 +1665,7 @@ function ReplicationApplierSuite () { //////////////////////////////////////////////////////////////////////////////// testStartApplierInvalidEndpoint1 : function () { - var state; - - state = replication.applier.state(); + var state = replication.applier.state(); assertFalse(state.state.running); @@ -1848,9 +1714,7 @@ function ReplicationApplierSuite () { //////////////////////////////////////////////////////////////////////////////// testStartApplierInvalidEndpoint2 : function () { - var state; - - state = replication.applier.state(); + var state = replication.applier.state(); assertFalse(state.state.running); // configure && start @@ -1899,9 +1763,7 @@ function ReplicationApplierSuite () { //////////////////////////////////////////////////////////////////////////////// testStopApplier : function () { - var state; - - state = replication.applier.state(); + var state = replication.applier.state(); assertFalse(state.state.running); @@ -1936,9 +1798,7 @@ function ReplicationApplierSuite () { //////////////////////////////////////////////////////////////////////////////// testApplierProperties : function () { - var properties; - - properties = replication.applier.properties(); + var properties = replication.applier.properties(); assertEqual(600, properties.requestTimeout); assertEqual(10, properties.connectTimeout); @@ -2068,8 +1928,6 @@ function ReplicationApplierSuite () { //////////////////////////////////////////////////////////////////////////////// testApplierPropertiesChange : function () { - var state; - replication.applier.properties({ endpoint: "tcp://9.9.9.9:9999", connectTimeout: 2, @@ -2078,7 +1936,7 @@ function ReplicationApplierSuite () { }); replication.applier.start(); - state = replication.applier.state(); + var state = replication.applier.state(); assertTrue(state.state.running); try { @@ -2095,9 +1953,7 @@ function ReplicationApplierSuite () { //////////////////////////////////////////////////////////////////////////////// testStateApplier : function () { - var state; - - state = replication.applier.state(); + var state = replication.applier.state(); assertFalse(state.state.running); assertMatch(/^\d+-\d+-\d+T\d+:\d+:\d+Z$/, state.state.time); From 591851f9acc71f6692bbda4de7636f1bb8ac1a14 Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Thu, 7 Apr 2016 13:07:47 +0200 Subject: [PATCH 04/20] ported bugfix from 2.8 --- arangod/Wal/CollectorThread.cpp | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/arangod/Wal/CollectorThread.cpp b/arangod/Wal/CollectorThread.cpp index 0f551439e3..fb6ccc05b9 100644 --- a/arangod/Wal/CollectorThread.cpp +++ b/arangod/Wal/CollectorThread.cpp @@ -1223,6 +1223,15 @@ char* CollectorThread::nextFreeMarkerPosition( LOG_TOPIC(ERR, Logger::COLLECTOR) << "cannot select journal: '" << TRI_last_error() << "'"; goto leave; } + + // must rotate the existing journal. now update its stats + if (cache->lastFid > 0) { + auto& dfi = createDfi(cache, cache->lastFid); + document->_datafileStatistics.increaseUncollected(cache->lastFid, + dfi.numberUncollected); + // and reset afterwards + dfi.numberUncollected = 0; + } // journal is full, close it and sync LOG_TOPIC(DEBUG, Logger::COLLECTOR) << "closing full journal '" << datafile->getName(datafile) @@ -1230,15 +1239,6 @@ char* CollectorThread::nextFreeMarkerPosition( TRI_CloseDatafileDocumentCollection(document, i, false); } - // must rotate the existing journal. now update its stats - if (cache->lastFid > 0) { - auto& dfi = getDfi(cache, cache->lastFid); - document->_datafileStatistics.increaseUncollected(cache->lastFid, - dfi.numberUncollected); - // and reset afterwards - dfi.numberUncollected = 0; - } - datafile = TRI_CreateDatafileDocumentCollection(document, tick, targetSize, false); @@ -1256,6 +1256,9 @@ char* CollectorThread::nextFreeMarkerPosition( THROW_ARANGO_EXCEPTION(res); } + + cache->lastDatafile = datafile; + cache->lastFid = datafile->_fid; } // next iteration leave: From 8235fcadc20620d5f28b3bb84dc00f6881e69669 Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Thu, 7 Apr 2016 13:09:33 +0200 Subject: [PATCH 05/20] properly cast --- arangod/FulltextIndex/fulltext-index.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arangod/FulltextIndex/fulltext-index.cpp b/arangod/FulltextIndex/fulltext-index.cpp index f60cccb6fc..247baa55e7 100644 --- a/arangod/FulltextIndex/fulltext-index.cpp +++ b/arangod/FulltextIndex/fulltext-index.cpp @@ -398,7 +398,7 @@ static inline node_t** FollowersNodes(void* data) { uint8_t numAllocated = *head; uint8_t* keys = (uint8_t*)(head + 2); // numAllocated + numEntries - return (node_t**)(uint8_t*)((keys + numAllocated) + Padding(numAllocated)); + return reinterpret_cast(keys + numAllocated + Padding(numAllocated)); } //////////////////////////////////////////////////////////////////////////////// @@ -424,7 +424,7 @@ static inline node_t** FollowersNodesPos(void* data, uint32_t numAllocated) { uint8_t* head = (uint8_t*)data; uint8_t* keys = (uint8_t*)(head + 2); // numAllocated + numEntries - return (node_t**)(uint8_t*)((keys + numAllocated) + Padding(numAllocated)); + return reinterpret_cast(keys + numAllocated + Padding(numAllocated)); } //////////////////////////////////////////////////////////////////////////////// From c452cb00bca0a05f7395d04822af5953bc4b7391 Mon Sep 17 00:00:00 2001 From: hkernbach Date: Thu, 7 Apr 2016 12:54:16 +0200 Subject: [PATCH 06/20] issue #1793 --- .../frontend/js/collections/arangoDocument.js | 12 +++---- .../APP/frontend/js/routers/router.js | 12 ++++++- .../APP/frontend/js/views/documentsView.js | 36 +++++++++++++++++-- 3 files changed, 50 insertions(+), 10 deletions(-) diff --git a/js/apps/system/_admin/aardvark/APP/frontend/js/collections/arangoDocument.js b/js/apps/system/_admin/aardvark/APP/frontend/js/collections/arangoDocument.js index dc0770da7e..0a6b05ee82 100644 --- a/js/apps/system/_admin/aardvark/APP/frontend/js/collections/arangoDocument.js +++ b/js/apps/system/_admin/aardvark/APP/frontend/js/collections/arangoDocument.js @@ -11,7 +11,7 @@ window.arangoDocument = Backbone.Collection.extend({ cache: false, type: 'DELETE', contentType: "application/json", - url: "/_api/edge/" + colid + "/" + docid, + url: "/_api/edge/" + encodeURIComponent(colid) + "/" + encodeURIComponent(docid), success: function () { callback(false); }, @@ -25,7 +25,7 @@ window.arangoDocument = Backbone.Collection.extend({ cache: false, type: 'DELETE', contentType: "application/json", - url: "/_api/document/" + colid + "/" + docid, + url: "/_api/document/" + encodeURIComponent(colid) + "/" + encodeURIComponent(docid), success: function () { callback(false); }, @@ -116,7 +116,7 @@ window.arangoDocument = Backbone.Collection.extend({ $.ajax({ cache: false, type: "GET", - url: "/_api/edge/" + colid +"/"+ docid, + url: "/_api/edge/" + encodeURIComponent(colid) +"/"+ encodeURIComponent(docid), contentType: "application/json", processData: false, success: function(data) { @@ -134,7 +134,7 @@ window.arangoDocument = Backbone.Collection.extend({ $.ajax({ cache: false, type: "GET", - url: "/_api/document/" + colid +"/"+ docid, + url: "/_api/document/" + encodeURIComponent(colid) +"/"+ encodeURIComponent(docid), contentType: "application/json", processData: false, success: function(data) { @@ -150,7 +150,7 @@ window.arangoDocument = Backbone.Collection.extend({ $.ajax({ cache: false, type: "PUT", - url: "/_api/edge/" + colid + "/" + docid, + url: "/_api/edge/" + encodeURIComponent(colid) + "/" + encodeURIComponent(docid), data: model, contentType: "application/json", processData: false, @@ -166,7 +166,7 @@ window.arangoDocument = Backbone.Collection.extend({ $.ajax({ cache: false, type: "PUT", - url: "/_api/document/" + colid + "/" + docid, + url: "/_api/document/" + encodeURIComponent(colid) + "/" + encodeURIComponent(docid), data: model, contentType: "application/json", processData: false, diff --git a/js/apps/system/_admin/aardvark/APP/frontend/js/routers/router.js b/js/apps/system/_admin/aardvark/APP/frontend/js/routers/router.js index b74db032f6..fa65741fbd 100644 --- a/js/apps/system/_admin/aardvark/APP/frontend/js/routers/router.js +++ b/js/apps/system/_admin/aardvark/APP/frontend/js/routers/router.js @@ -258,7 +258,17 @@ }); } this.documentView.colid = colid; - this.documentView.docid = docid; + + var doc = window.location.hash.split("/")[2]; + + var test = (doc.split("%").length - 1) % 3; + + if (decodeURI(doc) !== doc && test !== 0) { + doc = decodeURIComponent(doc); + } + this.documentView.docid = doc; + + this.documentView.render(); var callback = function(error, type) { diff --git a/js/apps/system/_admin/aardvark/APP/frontend/js/views/documentsView.js b/js/apps/system/_admin/aardvark/APP/frontend/js/views/documentsView.js index c44db39b0e..d950f9d5cc 100644 --- a/js/apps/system/_admin/aardvark/APP/frontend/js/views/documentsView.js +++ b/js/apps/system/_admin/aardvark/APP/frontend/js/views/documentsView.js @@ -585,6 +585,7 @@ var from = $('.modal-body #new-edge-from-attr').last().val(); var to = $('.modal-body #new-edge-to').last().val(); var key = $('.modal-body #new-edge-key-attr').last().val(); + var url; var callback = function(error, data) { @@ -593,7 +594,15 @@ } else { window.modalView.hide(); - window.location.hash = "collection/" + data; + data = data.split('/'); + + try { + url = "collection/" + data[0] + '/' + data[1]; + decodeURI(url); + } catch (ex) { + url = "collection/" + data[0] + '/' + encodeURIComponent(data[1]); + } + window.location.hash = url; } }.bind(this); @@ -608,6 +617,7 @@ addDocument: function() { var collid = window.location.hash.split("/")[1]; var key = $('.modal-body #new-document-key-attr').last().val(); + var url; var callback = function(error, data) { if (error) { @@ -615,7 +625,16 @@ } else { window.modalView.hide(); - window.location.hash = "collection/" + data; + data = data.split('/'); + + try { + url = "collection/" + data[0] + '/' + data[1]; + decodeURI(url); + } catch (ex) { + url = "collection/" + data[0] + '/' + encodeURIComponent(data[1]); + } + + window.location.hash = url; } }.bind(this); @@ -862,7 +881,18 @@ clicked: function (event) { var self = event.currentTarget; - window.App.navigate("collection/" + this.collection.collectionID + "/" + $(self).attr("id").substr(4), true); + + var url, doc = $(self).attr("id").substr(4); + + try { + url = "collection/" + this.collection.collectionID + '/' + doc; + decodeURI(doc); + } catch (ex) { + url = "collection/" + this.collection.collectionID + '/' + encodeURIComponent(doc); + } + + //window.App.navigate(url, true); + window.location.hash = url; }, drawTable: function() { From f6c818a39950e0776119f95fe1f297e12628aa37 Mon Sep 17 00:00:00 2001 From: Frank Celler Date: Thu, 7 Apr 2016 14:15:18 +0200 Subject: [PATCH 07/20] removed tolower for NC --- arangod/HttpServer/HttpCommTask.cpp | 4 ++-- arangod/RestHandler/RestJobHandler.cpp | 2 +- lib/Rest/GeneralResponse.cpp | 2 -- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/arangod/HttpServer/HttpCommTask.cpp b/arangod/HttpServer/HttpCommTask.cpp index 75f1015c4a..ed74605485 100644 --- a/arangod/HttpServer/HttpCommTask.cpp +++ b/arangod/HttpServer/HttpCommTask.cpp @@ -531,7 +531,7 @@ bool HttpCommTask::processRead() { static std::string const wwwAuthenticate = "www-authenticate"; if (sendWwwAuthenticateHeader()) { - std::string const realm = + static std::string const realm = "basic realm=\"" + _server->handlerFactory()->authenticationRealm(_request) + "\""; @@ -772,7 +772,7 @@ void HttpCommTask::fillWriteBuffer() { //////////////////////////////////////////////////////////////////////////////// void HttpCommTask::processCorsOptions(uint32_t compatibility) { - std::string const allowedMethods = "DELETE, GET, HEAD, PATCH, POST, PUT"; + static std::string const allowedMethods = "DELETE, GET, HEAD, PATCH, POST, PUT"; HttpResponse response(GeneralResponse::ResponseCode::OK, compatibility); diff --git a/arangod/RestHandler/RestJobHandler.cpp b/arangod/RestHandler/RestJobHandler.cpp index 1029a569ec..580b2bb4cd 100644 --- a/arangod/RestHandler/RestJobHandler.cpp +++ b/arangod/RestHandler/RestJobHandler.cpp @@ -107,7 +107,7 @@ void RestJobHandler::putJob() { _response = response; // plus a new header - static std::string xArango = "x-arango-async-id"; + static std::string const xArango = "x-arango-async-id"; _response->setHeaderNC(xArango, value); } diff --git a/lib/Rest/GeneralResponse.cpp b/lib/Rest/GeneralResponse.cpp index ff5ae29dcc..4b07df38d2 100644 --- a/lib/Rest/GeneralResponse.cpp +++ b/lib/Rest/GeneralResponse.cpp @@ -456,8 +456,6 @@ void GeneralResponse::setHeader(std::string const& key, void GeneralResponse::setHeaderNC(std::string const& key, std::string const& value) { - std::string k = StringUtils::tolower(key); - _headers[key] = value; } From 32338ef07f166bbd910f452031953a6d7dc4edfd Mon Sep 17 00:00:00 2001 From: Wilfried Goesgens Date: Thu, 7 Apr 2016 14:26:44 +0200 Subject: [PATCH 08/20] Also properly detect the end of the header when the server only sends \n --- lib/SimpleHttpClient/SimpleHttpClient.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/SimpleHttpClient/SimpleHttpClient.cpp b/lib/SimpleHttpClient/SimpleHttpClient.cpp index 4b96e5aebd..e5c7701aa2 100644 --- a/lib/SimpleHttpClient/SimpleHttpClient.cpp +++ b/lib/SimpleHttpClient/SimpleHttpClient.cpp @@ -632,7 +632,7 @@ void SimpleHttpClient::processHeader() { } // end of header found - if (*ptr == '\r' || *ptr == '\0') { + if (*ptr == '\r' || *ptr == '\n' || *ptr == '\0') { size_t len = pos - ptr; _readBufferOffset += len + 1; ptr += len + 1; From 8a0cc60266ec7d978b1ddc957f5681a2a4c64b3b Mon Sep 17 00:00:00 2001 From: Wilfried Goesgens Date: Thu, 7 Apr 2016 14:51:01 +0200 Subject: [PATCH 09/20] i.e. github requires a useragent, thus enable it. --- lib/V8/v8-utils.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/V8/v8-utils.cpp b/lib/V8/v8-utils.cpp index 2d933f2fe6..44f36cacbe 100644 --- a/lib/V8/v8-utils.cpp +++ b/lib/V8/v8-utils.cpp @@ -808,7 +808,8 @@ static void JS_Download(v8::FunctionCallbackInfo const& args) { SimpleHttpClient client(connection.get(), timeout, false); client.setSupportDeflate(false); - client.setExposeArangoDB(false); + // security by obscurity won't work. Github requires a useragent nowadays. + client.setExposeArangoDB(true); v8::Handle result = v8::Object::New(isolate); From ac99d29dd4eeebdc893df1b86500756cb8eac4c4 Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Thu, 7 Apr 2016 16:39:53 +0200 Subject: [PATCH 10/20] fixed misusage --- lib/Rest/GeneralRequest.cpp | 2 +- lib/Rest/GeneralResponse.cpp | 2 +- lib/Rest/HttpRequest.cpp | 3 --- lib/Rest/HttpRequest.h | 4 ---- 4 files changed, 2 insertions(+), 9 deletions(-) diff --git a/lib/Rest/GeneralRequest.cpp b/lib/Rest/GeneralRequest.cpp index e4add46e16..2a2c4d733f 100644 --- a/lib/Rest/GeneralRequest.cpp +++ b/lib/Rest/GeneralRequest.cpp @@ -273,6 +273,6 @@ std::string const& GeneralRequest::value(std::string const& key, bool& found) co void GeneralRequest::setArrayValue(char* key, size_t length, char const* value) { std::string keyStr(key, length); - _arrayValues[key].emplace_back(value); + _arrayValues[keyStr].emplace_back(value); } diff --git a/lib/Rest/GeneralResponse.cpp b/lib/Rest/GeneralResponse.cpp index ff6ac9ecd1..4f9ea9be8e 100644 --- a/lib/Rest/GeneralResponse.cpp +++ b/lib/Rest/GeneralResponse.cpp @@ -450,7 +450,7 @@ void GeneralResponse::setHeader(std::string const& key, std::string const& value) { std::string k = StringUtils::tolower(key); - _headers[key] = value; + _headers[k] = value; } void GeneralResponse::setHeaderNC(std::string const& key, diff --git a/lib/Rest/HttpRequest.cpp b/lib/Rest/HttpRequest.cpp index 8ebbb9d837..4f7e0c30e5 100644 --- a/lib/Rest/HttpRequest.cpp +++ b/lib/Rest/HttpRequest.cpp @@ -750,6 +750,3 @@ std::shared_ptr HttpRequest::toVelocyPack( return parser.steal(); } -TRI_json_t* HttpRequest::toJson(char** errmsg) { - return TRI_Json2String(TRI_UNKNOWN_MEM_ZONE, body().c_str(), errmsg); -} diff --git a/lib/Rest/HttpRequest.h b/lib/Rest/HttpRequest.h index af44d5cba5..6dc3ac46ec 100644 --- a/lib/Rest/HttpRequest.h +++ b/lib/Rest/HttpRequest.h @@ -28,7 +28,6 @@ #include "Rest/GeneralRequest.h" #include "Basics/StringBuffer.h" -#include "Basics/json.h" #include "Endpoint/ConnectionInfo.h" namespace arangodb { @@ -74,9 +73,6 @@ class HttpRequest : public GeneralRequest { std::shared_ptr toVelocyPack( arangodb::velocypack::Options const*); - // the request body as TRI_json_t* - TRI_json_t* toJson(char**); - using GeneralRequest::setHeader; private: From 8a86c0132ed7491b4166bfe579c7c7d00f64c263 Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Thu, 7 Apr 2016 16:40:07 +0200 Subject: [PATCH 11/20] removed unused variables --- lib/Endpoint/Endpoint.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/Endpoint/Endpoint.cpp b/lib/Endpoint/Endpoint.cpp index 5134236dc6..092be6acfc 100644 --- a/lib/Endpoint/Endpoint.cpp +++ b/lib/Endpoint/Endpoint.cpp @@ -194,16 +194,13 @@ Endpoint* Endpoint::factory(const Endpoint::EndpointType type, } std::string copy = unifiedForm(specification); - std::string prefix = "http"; TransportType protocol = TransportType::HTTP; if (StringUtils::isPrefix(copy, "http+")) { protocol = TransportType::HTTP; - prefix = "http+"; copy = copy.substr(5); } else if (StringUtils::isPrefix(copy, "vpp+")) { protocol = TransportType::VPP; - prefix = "vpp+"; copy = copy.substr(4); } else { // invalid protocol From ce11474602146b6258d35a25bf36ace7d129ea85 Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Thu, 7 Apr 2016 16:40:21 +0200 Subject: [PATCH 12/20] jslint --- js/common/tests/replication/replication.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/js/common/tests/replication/replication.js b/js/common/tests/replication/replication.js index fac20b2ac5..8062a1c747 100644 --- a/js/common/tests/replication/replication.js +++ b/js/common/tests/replication/replication.js @@ -1362,7 +1362,7 @@ function ReplicationLoggerSuite () { //////////////////////////////////////////////////////////////////////////////// testLoggerTransactionWrite1 : function () { - var c = db._create(cn); + db._create(cn); var tick = getLastLogTick(); var actual = db._executeTransaction({ @@ -1385,7 +1385,6 @@ function ReplicationLoggerSuite () { testLoggerTransactionWrite2 : function () { var c = db._create(cn); - var tick = getLastLogTick(); var actual = db._executeTransaction({ From 46df140751425f0118863a462234723cfa2241e2 Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Thu, 7 Apr 2016 16:40:30 +0200 Subject: [PATCH 13/20] removed unused method --- lib/Endpoint/EndpointList.cpp | 19 ------------------- lib/Endpoint/EndpointList.h | 3 --- 2 files changed, 22 deletions(-) diff --git a/lib/Endpoint/EndpointList.cpp b/lib/Endpoint/EndpointList.cpp index 17331c1b25..ed4200b99e 100644 --- a/lib/Endpoint/EndpointList.cpp +++ b/lib/Endpoint/EndpointList.cpp @@ -119,25 +119,6 @@ std::vector EndpointList::all() const { return result; } -//////////////////////////////////////////////////////////////////////////////// -/// @brief return all endpoints with a certain prefix -//////////////////////////////////////////////////////////////////////////////// - -std::map EndpointList::getByPrefix( - std::string const& prefix) const { - std::map result; - - for (auto& it : _endpoints) { - std::string const& key = it.first; - - if (StringUtils::isPrefix(key, prefix)) { - result[key] = it.second; - } - } - - return result; -} - //////////////////////////////////////////////////////////////////////////////// /// @brief return all endpoints with a certain encryption type //////////////////////////////////////////////////////////////////////////////// diff --git a/lib/Endpoint/EndpointList.h b/lib/Endpoint/EndpointList.h index 71bf3f9563..53b8c75e1e 100644 --- a/lib/Endpoint/EndpointList.h +++ b/lib/Endpoint/EndpointList.h @@ -48,9 +48,6 @@ class EndpointList { bool hasSsl() const; void dump() const; - private: - std::map getByPrefix(std::string const&) const; - private: std::map _endpoints; }; From d65662fb62b82b3be3c075ac0a3e1ed8575cf6dd Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Thu, 7 Apr 2016 16:41:35 +0200 Subject: [PATCH 14/20] pacify cppcheck --- arangod/Agency/ApplicationAgency.h | 3 +-- arangod/Cluster/AgencyComm.h | 2 +- arangod/Cluster/ApplicationCluster.cpp | 3 +-- arangod/Cluster/ClusterMethods.cpp | 4 +++- arangod/V8Server/v8-user-structures.cpp | 2 ++ arangod/V8Server/v8-vocbase.cpp | 1 + 6 files changed, 9 insertions(+), 6 deletions(-) diff --git a/arangod/Agency/ApplicationAgency.h b/arangod/Agency/ApplicationAgency.h index 45ed03d0cb..858b55640f 100644 --- a/arangod/Agency/ApplicationAgency.h +++ b/arangod/Agency/ApplicationAgency.h @@ -51,8 +51,7 @@ class ApplicationAgency : virtual public arangodb::rest::ApplicationFeature { public: - ApplicationAgency(ApplicationEndpointServer*); - + explicit ApplicationAgency(ApplicationEndpointServer*); ~ApplicationAgency(); diff --git a/arangod/Cluster/AgencyComm.h b/arangod/Cluster/AgencyComm.h index 79faae240f..c708652634 100644 --- a/arangod/Cluster/AgencyComm.h +++ b/arangod/Cluster/AgencyComm.h @@ -226,7 +226,7 @@ struct AgencyTransaction { ////////////////////////////////////////////////////////////////////////////// /// @brief shortcut to create a transaction with one operation ////////////////////////////////////////////////////////////////////////////// - AgencyTransaction(AgencyOperation operation) { + explicit AgencyTransaction(AgencyOperation const& operation) { operations.push_back(operation); } }; diff --git a/arangod/Cluster/ApplicationCluster.cpp b/arangod/Cluster/ApplicationCluster.cpp index a4d88bd74f..b761b927d6 100644 --- a/arangod/Cluster/ApplicationCluster.cpp +++ b/arangod/Cluster/ApplicationCluster.cpp @@ -375,11 +375,10 @@ bool ApplicationCluster::open() { AgencyComm comm; AgencyCommResult result; - bool success; do { AgencyCommLocker locker("Current", "WRITE"); - success = locker.successful(); + bool success = locker.successful(); if (success) { VPackBuilder builder; try { diff --git a/arangod/Cluster/ClusterMethods.cpp b/arangod/Cluster/ClusterMethods.cpp index 913b515366..58aa376375 100644 --- a/arangod/Cluster/ClusterMethods.cpp +++ b/arangod/Cluster/ClusterMethods.cpp @@ -670,6 +670,7 @@ int createDocumentOnCoordinator( } responseCode = res.answer_code; + TRI_ASSERT(res.answer != nullptr); auto parsedResult = res.answer->toVelocyPack(&VPackOptions::Defaults); resultBody.swap(parsedResult); return TRI_ERROR_NO_ERROR; @@ -701,6 +702,7 @@ int createDocumentOnCoordinator( } resultMap.emplace(res.shardID, tmpBuilder); } else { + TRI_ASSERT(res.answer != nullptr); resultMap.emplace(res.shardID, res.answer->toVelocyPack(&VPackOptions::Defaults)); auto resultHeaders = res.answer->headers(); @@ -1243,7 +1245,7 @@ int getFilteredDocumentsOnCoordinator( size_t resCount = TRI_LengthArrayJson(documents); for (size_t k = 0; k < resCount; ++k) { try { - TRI_json_t* element = TRI_LookupArrayJson(documents, k); + TRI_json_t const* element = TRI_LookupArrayJson(documents, k); std::string id = arangodb::basics::JsonHelper::checkAndGetStringValue( element, TRI_VOC_ATTRIBUTE_ID); auto tmpBuilder = basics::JsonHelper::toVelocyPack(element); diff --git a/arangod/V8Server/v8-user-structures.cpp b/arangod/V8Server/v8-user-structures.cpp index d002feed9f..df5fb2c87d 100644 --- a/arangod/V8Server/v8-user-structures.cpp +++ b/arangod/V8Server/v8-user-structures.cpp @@ -562,6 +562,8 @@ class KeySpace { } } + TRI_ASSERT(dest != nullptr); + if (!TRI_IsArrayJson(dest->json)) { TRI_V8_THROW_EXCEPTION(TRI_ERROR_INTERNAL); } diff --git a/arangod/V8Server/v8-vocbase.cpp b/arangod/V8Server/v8-vocbase.cpp index 82959e9379..28497f6389 100644 --- a/arangod/V8Server/v8-vocbase.cpp +++ b/arangod/V8Server/v8-vocbase.cpp @@ -2620,6 +2620,7 @@ static void MapGetVocBase(v8::Local const name, if (collection != nullptr && collection->_cid == 0) { delete collection; + collection = nullptr; TRI_V8_RETURN(v8::Handle()); } } From 1855b74aaa0339efbcde83b7df0ba3e2d3d9037c Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Thu, 7 Apr 2016 16:44:36 +0200 Subject: [PATCH 15/20] added asserts --- arangod/RestHandler/RestDocumentHandler.cpp | 2 ++ arangod/VocBase/collection.cpp | 2 +- arangod/VocBase/collection.h | 10 ++++++---- arangod/VocBase/document-collection.cpp | 12 +++++++----- arangod/VocBase/document-collection.h | 4 +++- 5 files changed, 19 insertions(+), 11 deletions(-) diff --git a/arangod/RestHandler/RestDocumentHandler.cpp b/arangod/RestHandler/RestDocumentHandler.cpp index aa4708a66d..c9804bf437 100644 --- a/arangod/RestHandler/RestDocumentHandler.cpp +++ b/arangod/RestHandler/RestDocumentHandler.cpp @@ -573,6 +573,7 @@ bool RestDocumentHandler::deleteDocument() { search = builder.slice(); } else { try { + TRI_ASSERT(_request != nullptr); builderPtr = _request->toVelocyPack(transactionContext->getVPackOptions()); } catch (...) { // If an error occurs here the body is not parsable. Fail with bad parameter @@ -636,6 +637,7 @@ bool RestDocumentHandler::readManyDocuments() { return false; } + TRI_ASSERT(_request != nullptr); auto builderPtr = _request->toVelocyPack(transactionContext->getVPackOptions()); VPackSlice search = builderPtr->slice(); diff --git a/arangod/VocBase/collection.cpp b/arangod/VocBase/collection.cpp index 7f31ae96c8..e2fece2630 100644 --- a/arangod/VocBase/collection.cpp +++ b/arangod/VocBase/collection.cpp @@ -828,7 +828,7 @@ TRI_collection_t* TRI_CreateCollection( // create collection structure if (collection == nullptr) { try { - TRI_collection_t* tmp = new TRI_collection_t(parameters); + TRI_collection_t* tmp = new TRI_collection_t(vocbase, parameters); collection = tmp; } catch (std::exception&) { collection = nullptr; diff --git a/arangod/VocBase/collection.h b/arangod/VocBase/collection.h index c105a02518..862e44d0ec 100644 --- a/arangod/VocBase/collection.h +++ b/arangod/VocBase/collection.h @@ -294,11 +294,13 @@ struct TRI_collection_t { TRI_collection_t(TRI_collection_t const&) = delete; TRI_collection_t& operator=(TRI_collection_t const&) = delete; - TRI_collection_t() - : _tickMax(0), _state(TRI_COL_STATE_WRITE), _lastError(0) {} + TRI_collection_t() = delete; + + explicit TRI_collection_t(TRI_vocbase_t* vocbase) + : _vocbase(vocbase), _tickMax(0), _state(TRI_COL_STATE_WRITE), _lastError(0) {} - explicit TRI_collection_t(arangodb::VocbaseCollectionInfo const& info) - : _info(info), _tickMax(0), _state(TRI_COL_STATE_WRITE), _lastError(0) {} + TRI_collection_t(TRI_vocbase_t* vocbase, arangodb::VocbaseCollectionInfo const& info) + : _info(info), _vocbase(vocbase), _tickMax(0), _state(TRI_COL_STATE_WRITE), _lastError(0) {} ~TRI_collection_t() = default; diff --git a/arangod/VocBase/document-collection.cpp b/arangod/VocBase/document-collection.cpp index 904d158000..c60748329e 100644 --- a/arangod/VocBase/document-collection.cpp +++ b/arangod/VocBase/document-collection.cpp @@ -68,8 +68,9 @@ using namespace arangodb::basics; /// @brief create a document collection //////////////////////////////////////////////////////////////////////////////// -TRI_document_collection_t::TRI_document_collection_t() - : _lock(), +TRI_document_collection_t::TRI_document_collection_t(TRI_vocbase_t* vocbase) + : TRI_collection_t(vocbase), + _lock(), _nextCompactionStartIndex(0), _lastCompactionStatus(nullptr), _useSecondaryIndexes(true), @@ -1209,7 +1210,7 @@ TRI_document_collection_t* TRI_CreateDocumentCollection( // first create the document collection TRI_document_collection_t* document; try { - document = new TRI_document_collection_t(); + document = new TRI_document_collection_t(vocbase); } catch (std::exception&) { document = nullptr; } @@ -1753,7 +1754,7 @@ TRI_document_collection_t* TRI_OpenDocumentCollection(TRI_vocbase_t* vocbase, // first open the document collection TRI_document_collection_t* document = nullptr; try { - document = new TRI_document_collection_t(); + document = new TRI_document_collection_t(vocbase); } catch (std::exception&) { } @@ -3707,7 +3708,7 @@ int TRI_document_collection_t::remove(arangodb::Transaction* trx, TRI_ASSERT(marker == nullptr); // get the header pointer of the previous revision - TRI_doc_mptr_t* oldHeader; + TRI_doc_mptr_t* oldHeader = nullptr; VPackSlice key; if (slice.isString()) { key = slice; @@ -3720,6 +3721,7 @@ int TRI_document_collection_t::remove(arangodb::Transaction* trx, return res; } + TRI_ASSERT(oldHeader != nullptr); prevRev = oldHeader->revisionIdAsSlice(); previous = *oldHeader; diff --git a/arangod/VocBase/document-collection.h b/arangod/VocBase/document-collection.h index dad11c4731..34d6d7625c 100644 --- a/arangod/VocBase/document-collection.h +++ b/arangod/VocBase/document-collection.h @@ -36,6 +36,8 @@ #include "VocBase/voc-types.h" #include "Wal/Marker.h" +struct TRI_vocbase_t; + namespace arangodb { class EdgeIndex; class Index; @@ -90,7 +92,7 @@ struct TRI_doc_collection_info_t { //////////////////////////////////////////////////////////////////////////////// struct TRI_document_collection_t : public TRI_collection_t { - TRI_document_collection_t(); + explicit TRI_document_collection_t(TRI_vocbase_t* vocbase); ~TRI_document_collection_t(); From 9d831326d452c9332bff0da82ea685b079e16868 Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Thu, 7 Apr 2016 16:51:05 +0200 Subject: [PATCH 16/20] fixed leaks in geo index --- UnitTests/Geo/georeg.cpp | 142 ++++++++++++++++++---------------- arangod/GeoIndex/GeoIndex.cpp | 35 +++++---- 2 files changed, 93 insertions(+), 84 deletions(-) diff --git a/UnitTests/Geo/georeg.cpp b/UnitTests/Geo/georeg.cpp index 5b2569c54a..48126604c1 100644 --- a/UnitTests/Geo/georeg.cpp +++ b/UnitTests/Geo/georeg.cpp @@ -316,6 +316,7 @@ BOOST_AUTO_TEST_CASE (tst_geo1000) { gcmass(1009,list1,5, 53245966); list1 = GeoIndex_ReadCursor(gcr,5); gcmass(1010,list1,4, 86589238); + GeoIndex_CursorFree(gcr); MyFree(gi); } @@ -365,6 +366,8 @@ gcp.longitude= 25.5; gcr = GeoIndex_NewCursor(gi,&gcp); list1 = GeoIndex_ReadCursor(gcr,1); icheck(11,1,list1->length); +GeoIndex_CoordinatesFree(list1); +GeoIndex_CursorFree(gcr); gcp.latitude = 89.9; gcp.longitude = -180.0; gcp.data = ix + 64; @@ -394,6 +397,7 @@ gcp.latitude = 89.9; gcp.longitude = -180.0; gcp.data = ix + 64; GeoIndex_insert(gi,&gcp); +GeoIndex_CoordinatesFree(list1); list1 = GeoIndex_NearestCountPoints(gi,&gcp,1); gccheck(13,list1, 1,"AAAAAAAAAAAAAAAABAAAAAAAA"); gicheck(14,gi); @@ -541,37 +545,37 @@ MyFree(gi); /* in some chaotic ways */ BOOST_AUTO_TEST_CASE (tst_geo70) { -gi=GeoIndex_new(); + gi=GeoIndex_new(); -gcp.latitude = 0.0; -gcp.longitude = 40.0; -gcp.data = &ix[4]; -i = GeoIndex_insert(gi,&gcp); -icheck(70,0,i); + gcp.latitude = 0.0; + gcp.longitude = 40.0; + gcp.data = &ix[4]; + i = GeoIndex_insert(gi,&gcp); + icheck(70,0,i); -gcp.data = &ix[5]; -i = GeoIndex_remove(gi,&gcp); -icheck(71,-1,i); + gcp.data = &ix[5]; + i = GeoIndex_remove(gi,&gcp); + icheck(71,-1,i); -gcp.longitude = 40.000001; -gcp.data = &ix[4]; -i = GeoIndex_remove(gi,&gcp); -icheck(72,-1,i); + gcp.longitude = 40.000001; + gcp.data = &ix[4]; + i = GeoIndex_remove(gi,&gcp); + icheck(72,-1,i); -gcp.latitude = 0.0000000001; -gcp.longitude = 40.0; -i = GeoIndex_remove(gi,&gcp); -icheck(73,-1,i); + gcp.latitude = 0.0000000001; + gcp.longitude = 40.0; + i = GeoIndex_remove(gi,&gcp); + icheck(73,-1,i); -gcp.latitude = 0.0; -i = GeoIndex_remove(gi,&gcp); -icheck(74,0,i); + gcp.latitude = 0.0; + i = GeoIndex_remove(gi,&gcp); + icheck(74,0,i); -i = GeoIndex_remove(gi,&gcp); -icheck(75,-1,i); + i = GeoIndex_remove(gi,&gcp); + icheck(75,-1,i); -for(j=1;j<=8;j++) -{ + for(j=1;j<=8;j++) + { gcp.latitude = 0.0; lo=j; lo=lo*10; @@ -579,73 +583,76 @@ for(j=1;j<=8;j++) gcp.data = &ix[j]; i = GeoIndex_insert(gi,&gcp); icheck(76,0,i); -} + } -gcp.latitude = 0.0; -gcp.longitude= 25.5; -list1 = GeoIndex_NearestCountPoints(gi,&gcp,1); -icheck(77,1,list1->length); -dcheck(78,0.0,list1->coordinates[0].latitude,0.0); -dcheck(79,30.0,list1->coordinates[0].longitude,0.0); -pcheck(80,&ix[3],(char *)list1->coordinates[0].data); -gcp.longitude= 24.5; -list1 = GeoIndex_NearestCountPoints(gi,&gcp,1); -icheck(81,1,list1->length); -dcheck(82,0.0,list1->coordinates[0].latitude,0.0); -dcheck(83,20.0,list1->coordinates[0].longitude,0.0); -pcheck(84,&ix[2],(char *)list1->coordinates[0].data); + gcp.latitude = 0.0; + gcp.longitude= 25.5; + list1 = GeoIndex_NearestCountPoints(gi,&gcp,1); + icheck(77,1,list1->length); + dcheck(78,0.0,list1->coordinates[0].latitude,0.0); + dcheck(79,30.0,list1->coordinates[0].longitude,0.0); + pcheck(80,&ix[3],(char *)list1->coordinates[0].data); + gcp.longitude= 24.5; + GeoIndex_CoordinatesFree(list1); + list1 = GeoIndex_NearestCountPoints(gi,&gcp,1); + icheck(81,1,list1->length); + dcheck(82,0.0,list1->coordinates[0].latitude,0.0); + dcheck(83,20.0,list1->coordinates[0].longitude,0.0); + pcheck(84,&ix[2],(char *)list1->coordinates[0].data); + GeoIndex_CoordinatesFree(list1); -gcp.latitude = 1.0; -gcp.longitude = 40.0; -gcp.data = &ix[14]; -i = GeoIndex_insert(gi,&gcp); -icheck(85,0,i); + gcp.latitude = 1.0; + gcp.longitude = 40.0; + gcp.data = &ix[14]; + i = GeoIndex_insert(gi,&gcp); + icheck(85,0,i); -gcp.longitude = 8000.0; -i = GeoIndex_insert(gi,&gcp); -icheck(86,-3,i); + gcp.longitude = 8000.0; + i = GeoIndex_insert(gi,&gcp); + icheck(86,-3,i); -gcp.latitude = 800.0; -gcp.longitude = 80.0; -i = GeoIndex_insert(gi,&gcp); -icheck(86,-3,i); + gcp.latitude = 800.0; + gcp.longitude = 80.0; + i = GeoIndex_insert(gi,&gcp); + icheck(86,-3,i); -gcp.latitude = 800.0; -gcp.longitude = 80.0; -i = GeoIndex_remove(gi,&gcp); -icheck(87,-3,i); + gcp.latitude = 800.0; + gcp.longitude = 80.0; + i = GeoIndex_remove(gi,&gcp); + icheck(87,-3,i); -gcp.latitude = 1.0; -gcp.longitude = 40.0; -gcp.data = &ix[14]; -i = GeoIndex_remove(gi,&gcp); -icheck(88,0,i); + gcp.latitude = 1.0; + gcp.longitude = 40.0; + gcp.data = &ix[14]; + i = GeoIndex_remove(gi,&gcp); + icheck(88,0,i); -for(j=1;j<10;j++) -{ + for(j=1;j<10;j++) + { gcp.latitude = 0.0; gcp.longitude = 40.0; gcp.data = &ix[20+j]; i = GeoIndex_insert(gi,&gcp); icheck(89,0,i); -} + } -for(j=1;j<10;j++) -{ + for(j=1;j<10;j++) + { gcp.latitude = 0.0; gcp.longitude = 40.0; gcp.data = &ix[20+j]; i = GeoIndex_remove(gi,&gcp); icheck(90,0,i); -} + } -gcp.latitude = 0.0; -gcp.longitude= 35.5; + gcp.latitude = 0.0; + gcp.longitude= 35.5; list1 = GeoIndex_NearestCountPoints(gi,&gcp,1); icheck(91,1,list1->length); dcheck(92,0.0,list1->coordinates[0].latitude,0.0); dcheck(93,40.0,list1->coordinates[0].longitude,0.0); pcheck(94,&ix[4],(char *)list1->coordinates[0].data); + GeoIndex_CoordinatesFree(list1); list1 = GeoIndex_NearestCountPoints(gi,&gcp,10); gccheck(95,list1, 8,"OPBAAAAAAAAAAAAAAAAAAAAAA"); @@ -890,6 +897,7 @@ BOOST_AUTO_TEST_CASE (tst_geo200) { } } } + GeoIndex_CoordinatesFree(list1); list1 = GeoIndex_PointsWithinRadius(gi,&gcp1,13000.0); if(list1->length==5) diff --git a/arangod/GeoIndex/GeoIndex.cpp b/arangod/GeoIndex/GeoIndex.cpp index 8375ff688f..18c13e5dc3 100644 --- a/arangod/GeoIndex/GeoIndex.cpp +++ b/arangod/GeoIndex/GeoIndex.cpp @@ -1968,6 +1968,9 @@ int GeoIndex_remove(GeoIndex* gi, GeoCoordinate* c) { /* user when the results of a search are finished with */ /* =================================================== */ void GeoIndex_CoordinatesFree(GeoCoordinates* clist) { + if (clist == nullptr) { + return; + } TRI_Free(TRI_UNKNOWN_MEM_ZONE, clist->coordinates); TRI_Free(TRI_UNKNOWN_MEM_ZONE, clist->distances); TRI_Free(TRI_UNKNOWN_MEM_ZONE, clist); @@ -1993,10 +1996,7 @@ typedef struct { } hpot; // pot for putting on the heap bool hpotcompare(hpot a, hpot b) { - if (a.dist > b.dist) - return true; - else - return false; + return (a.dist > b.dist); } typedef struct { @@ -2036,17 +2036,20 @@ GeoFix makedist(GeoPot* pot, GeoDetailedPoint* gd) { GeoCursor* GeoIndex_NewCursor(GeoIndex* gi, GeoCoordinate* c) { GeoIx* gix; - GeoCr* gcr; hpot hp; - if (c->longitude < -180.0) return NULL; - if (c->longitude > 180.0) return NULL; - if (c->latitude < -90.0) return NULL; - if (c->latitude > 90.0) return NULL; + if (c->longitude < -180.0) return nullptr; + if (c->longitude > 180.0) return nullptr; + if (c->latitude < -90.0) return nullptr; + if (c->latitude > 90.0) return nullptr; gix = (GeoIx*)gi; - gcr = static_cast( - TRI_Allocate(TRI_UNKNOWN_MEM_ZONE, sizeof(GeoCr), false)); + GeoCr* gcr = nullptr; + + try { + gcr = new GeoCr; + } + catch (...) { } - if (gcr == NULL) { + if (gcr == nullptr) { return (GeoCursor*)gcr; } gcr->Ix = gix; @@ -2145,13 +2148,11 @@ GeoCoordinates* GeoIndex_ReadCursor(GeoCursor* gc, int count) { } void GeoIndex_CursorFree(GeoCursor* gc) { - GeoCr* cr; - if (gc == NULL) { + if (gc == nullptr) { return; } - cr = (GeoCr*)gc; - TRI_Free(TRI_UNKNOWN_MEM_ZONE, cr); - return; + GeoCr* cr = reinterpret_cast(gc); + delete cr; } /* =================================================== */ From 32c0c919262a0e14835765f2d8b3e514bc8011fa Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Thu, 7 Apr 2016 17:01:45 +0200 Subject: [PATCH 17/20] fixed tests --- js/server/tests/aql/aql-failures-noncluster.js | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/js/server/tests/aql/aql-failures-noncluster.js b/js/server/tests/aql/aql-failures-noncluster.js index 5579011b8a..f6394c9f41 100644 --- a/js/server/tests/aql/aql-failures-noncluster.js +++ b/js/server/tests/aql/aql-failures-noncluster.js @@ -205,7 +205,7 @@ function ahuacatlFailureSuite () { testReturnBlock : function () { internal.debugSetFailAt("ReturnBlock::getSome"); - assertFailingQuery("FOR year IN [ 2010, 2011, 2012 ] LET quarters = ((FOR q IN [ 'jhaskdjhjkasdhkjahsd', 2, 3, 4 ] RETURN q)) RETURN 'kljhasdjkhaskjdhaskjdhasd'"); + assertFailingQuery("FOR year IN [ 2010, 2011, 2012 ] LET quarters = ((FOR q IN [ 'jhaskdjhjkasdhkjahsd', 2, 3, 4 ] RETURN CONCAT('xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx', q))) RETURN LENGTH(quarters)"); }, //////////////////////////////////////////////////////////////////////////////// @@ -255,9 +255,10 @@ function ahuacatlFailureSuite () { testSortBlock5 : function () { internal.debugSetFailAt("SortBlock::doSortingNext2"); - assertFailingQuery("FOR i IN " + c.name() + " COLLECT key = i._key SORT key RETURN key"); - assertFailingQuery("FOR i IN " + c.name() + " COLLECT key = i.value SORT key RETURN key"); - assertFailingQuery("FOR i IN " + c.name() + " COLLECT key = i.value2 SORT key RETURN key"); + // we need values that are >= 16 bytes long + assertFailingQuery("FOR i IN " + c.name() + " COLLECT key = i._key SORT CONCAT('xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx', key) RETURN key"); + assertFailingQuery("FOR i IN " + c.name() + " COLLECT key = i.value SORT CONCAT('xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx', key) RETURN key"); + assertFailingQuery("FOR i IN " + c.name() + " COLLECT key = i.value2 SORT CONCAT('xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx', key) RETURN key"); }, //////////////////////////////////////////////////////////////////////////////// @@ -546,7 +547,7 @@ function ahuacatlFailureSuite () { assertFailingQuery("FOR i IN " + c.name() + " FILTER 1 IN i.value[*] RETURN i"); }, - testIndexNodeSkiplist9 : function () { + testIndexNodeSkiplist6 : function () { c.ensureSkiplist("value"); internal.debugSetFailAt("SkiplistIndex::accessFitsIndex"); assertFailingQuery("FOR i IN " + c.name() + " FILTER i.value == 1 RETURN i"); From 6df72216919713fff16ffdc038a73a87e678db9a Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Thu, 7 Apr 2016 17:08:44 +0200 Subject: [PATCH 18/20] cleanup --- arangod/Aql/Ast.cpp | 1 - arangod/Aql/ExecutionEngine.cpp | 1 - arangod/Aql/ExecutionNode.cpp | 1 + arangod/Aql/ExecutionPlan.cpp | 1 - js/common/bootstrap/errors.js | 1 - .../shell/shell-transactions-noncluster.js | 86 ------------------- lib/Basics/errors.dat | 1 - lib/Basics/voc-errors.cpp | 1 - lib/Basics/voc-errors.h | 12 --- 9 files changed, 1 insertion(+), 104 deletions(-) diff --git a/arangod/Aql/Ast.cpp b/arangod/Aql/Ast.cpp index f8ea5290c0..e919d3d97b 100644 --- a/arangod/Aql/Ast.cpp +++ b/arangod/Aql/Ast.cpp @@ -145,7 +145,6 @@ Ast::~Ast() {} //////////////////////////////////////////////////////////////////////////////// TRI_json_t* Ast::toJson(TRI_memory_zone_t* zone, bool verbose) const { -#warning Deprecated TRI_json_t* json = TRI_CreateArrayJson(zone); if (json == nullptr) { diff --git a/arangod/Aql/ExecutionEngine.cpp b/arangod/Aql/ExecutionEngine.cpp index 0f0e2e5f29..afbfb2aea0 100644 --- a/arangod/Aql/ExecutionEngine.cpp +++ b/arangod/Aql/ExecutionEngine.cpp @@ -472,7 +472,6 @@ struct CoordinatorInstanciator : public WalkerWorker { EngineInfo const& info, Collection* collection, QueryId& connectedId, std::string const& shardId, TRI_json_t* jsonPlan) { -#warning still Json inplace. Needs to be fixed // create a JSON representation of the plan Json result(Json::Object); diff --git a/arangod/Aql/ExecutionNode.cpp b/arangod/Aql/ExecutionNode.cpp index d12eec5ba2..7ac060b222 100644 --- a/arangod/Aql/ExecutionNode.cpp +++ b/arangod/Aql/ExecutionNode.cpp @@ -1,3 +1,4 @@ +//////////////////////////////////////////////////////////////////////////////// /// @brief Infrastructure for ExecutionPlans /// /// DISCLAIMER diff --git a/arangod/Aql/ExecutionPlan.cpp b/arangod/Aql/ExecutionPlan.cpp index 4879292acf..d6d27dc958 100644 --- a/arangod/Aql/ExecutionPlan.cpp +++ b/arangod/Aql/ExecutionPlan.cpp @@ -220,7 +220,6 @@ ExecutionPlan* ExecutionPlan::clone(Query const& query) { arangodb::basics::Json ExecutionPlan::toJson(Ast* ast, TRI_memory_zone_t* zone, bool verbose) const { -#warning Remove this // TODO VPackBuilder b; _root->toVelocyPack(b, verbose); diff --git a/js/common/bootstrap/errors.js b/js/common/bootstrap/errors.js index 3b4c397a7b..54c9444326 100644 --- a/js/common/bootstrap/errors.js +++ b/js/common/bootstrap/errors.js @@ -47,7 +47,6 @@ "ERROR_HTTP_CORRUPTED_JSON" : { "code" : 600, "message" : "invalid JSON object" }, "ERROR_HTTP_SUPERFLUOUS_SUFFICES" : { "code" : 601, "message" : "superfluous URL suffices" }, "ERROR_ARANGO_ILLEGAL_STATE" : { "code" : 1000, "message" : "illegal state" }, - "ERROR_ARANGO_SHAPER_FAILED" : { "code" : 1001, "message" : "could not shape document" }, "ERROR_ARANGO_DATAFILE_SEALED" : { "code" : 1002, "message" : "datafile sealed" }, "ERROR_ARANGO_UNKNOWN_COLLECTION_TYPE" : { "code" : 1003, "message" : "unknown type" }, "ERROR_ARANGO_READ_ONLY" : { "code" : 1004, "message" : "read only" }, diff --git a/js/server/tests/shell/shell-transactions-noncluster.js b/js/server/tests/shell/shell-transactions-noncluster.js index b7faf07e92..80b36cca6f 100644 --- a/js/server/tests/shell/shell-transactions-noncluster.js +++ b/js/server/tests/shell/shell-transactions-noncluster.js @@ -4936,92 +4936,6 @@ function transactionServerFailuresSuite () { testHelper.waitUnload(c); assertEqual(100, c.count()); - }, - -//////////////////////////////////////////////////////////////////////////////// -/// @brief test: cannot write attribute marker for trx -//////////////////////////////////////////////////////////////////////////////// - - testNoAttributeMarker : function () { - internal.debugClearFailAt(); - - db._drop(cn); - c = db._create(cn); - - var i; - for (i = 0; i < 100; ++i) { - c.save({ _key: "test" + i, a: i }); - } - assertEqual(100, c.count()); - - internal.wal.flush(true, true); - - try { - TRANSACTION({ - collections: { - write: [ cn ], - }, - action: function () { - var i; - for (i = 100; i < 200; ++i) { - c.save({ _key: "test" + i, a: i }); - } - - internal.debugSetFailAt("ShaperWriteAttributeMarker"); - c.save({ _key: "test100", newAttribute: "foo" }); - } - }); - fail(); - } - catch (err) { - assertEqual(internal.errors.ERROR_ARANGO_SHAPER_FAILED.code, err.errorNum); - } - - assertEqual(100, c.count()); - internal.debugClearFailAt(); - }, - -//////////////////////////////////////////////////////////////////////////////// -/// @brief test: cannot write shape marker for trx -//////////////////////////////////////////////////////////////////////////////// - - testNoShapeMarker : function () { - internal.debugClearFailAt(); - - db._drop(cn); - c = db._create(cn); - - var i; - for (i = 0; i < 100; ++i) { - c.save({ _key: "test" + i, a: i }); - } - assertEqual(100, c.count()); - - internal.wal.flush(true, true); - - try { - TRANSACTION({ - collections: { - write: [ cn ], - }, - action: function () { - var i; - for (i = 100; i < 200; ++i) { - c.save({ _key: "test" + i, a: i }); - } - - internal.debugSetFailAt("ShaperWriteShapeMarker"); - c.save({ _key: "test100", newAttribute: "foo", reallyNew: "foo" }); - } - }); - fail(); - } - catch (err) { - assertEqual(internal.errors.ERROR_ARANGO_SHAPER_FAILED.code, err.errorNum); - } - - assertEqual(100, c.count()); - internal.debugClearFailAt(); } }; diff --git a/lib/Basics/errors.dat b/lib/Basics/errors.dat index 6ad720479a..5049944eff 100755 --- a/lib/Basics/errors.dat +++ b/lib/Basics/errors.dat @@ -55,7 +55,6 @@ ERROR_HTTP_SUPERFLUOUS_SUFFICES,601,"superfluous URL suffices","Will be raised w ################################################################################ ERROR_ARANGO_ILLEGAL_STATE,1000,"illegal state","Internal error that will be raised when the datafile is not in the required state." -ERROR_ARANGO_SHAPER_FAILED,1001,"could not shape document","Internal error that will be raised when the shaper encountered a problem." ERROR_ARANGO_DATAFILE_SEALED,1002,"datafile sealed","Internal error that will be raised when trying to write to a datafile." ERROR_ARANGO_UNKNOWN_COLLECTION_TYPE,1003,"unknown type","Internal error that will be raised when an unknown collection type is encountered." ERROR_ARANGO_READ_ONLY,1004,"read only","Internal error that will be raised when trying to write to a read-only datafile or collection." diff --git a/lib/Basics/voc-errors.cpp b/lib/Basics/voc-errors.cpp index 557144058a..77c2565f00 100644 --- a/lib/Basics/voc-errors.cpp +++ b/lib/Basics/voc-errors.cpp @@ -43,7 +43,6 @@ void TRI_InitializeErrorMessages () { REG_ERROR(ERROR_HTTP_CORRUPTED_JSON, "invalid JSON object"); REG_ERROR(ERROR_HTTP_SUPERFLUOUS_SUFFICES, "superfluous URL suffices"); REG_ERROR(ERROR_ARANGO_ILLEGAL_STATE, "illegal state"); - REG_ERROR(ERROR_ARANGO_SHAPER_FAILED, "could not shape document"); REG_ERROR(ERROR_ARANGO_DATAFILE_SEALED, "datafile sealed"); REG_ERROR(ERROR_ARANGO_UNKNOWN_COLLECTION_TYPE, "unknown type"); REG_ERROR(ERROR_ARANGO_READ_ONLY, "read only"); diff --git a/lib/Basics/voc-errors.h b/lib/Basics/voc-errors.h index 3371082ebe..bb3558085e 100644 --- a/lib/Basics/voc-errors.h +++ b/lib/Basics/voc-errors.h @@ -83,8 +83,6 @@ /// - 1000: @LIT{illegal state} /// Internal error that will be raised when the datafile is not in the /// required state. -/// - 1001: @LIT{could not shape document} -/// Internal error that will be raised when the shaper encountered a problem. /// - 1002: @LIT{datafile sealed} /// Internal error that will be raised when trying to write to a datafile. /// - 1003: @LIT{unknown type} @@ -1010,16 +1008,6 @@ void TRI_InitializeErrorMessages (); #define TRI_ERROR_ARANGO_ILLEGAL_STATE (1000) -//////////////////////////////////////////////////////////////////////////////// -/// @brief 1001: ERROR_ARANGO_SHAPER_FAILED -/// -/// could not shape document -/// -/// Internal error that will be raised when the shaper encountered a problem. -//////////////////////////////////////////////////////////////////////////////// - -#define TRI_ERROR_ARANGO_SHAPER_FAILED (1001) - //////////////////////////////////////////////////////////////////////////////// /// @brief 1002: ERROR_ARANGO_DATAFILE_SEALED /// From c2749b850f195723ce31b02c966b13c0cf53fcfd Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Thu, 7 Apr 2016 17:48:51 +0200 Subject: [PATCH 19/20] fixed tests --- arangod/Utils/Transaction.cpp | 21 ++++-- .../shell/shell-transactions-noncluster.js | 65 +++++++------------ 2 files changed, 39 insertions(+), 47 deletions(-) diff --git a/arangod/Utils/Transaction.cpp b/arangod/Utils/Transaction.cpp index 74fb977933..72a2839fe2 100644 --- a/arangod/Utils/Transaction.cpp +++ b/arangod/Utils/Transaction.cpp @@ -2101,8 +2101,14 @@ bool Transaction::supportsFilterCondition( arangodb::aql::AstNode const* condition, arangodb::aql::Variable const* reference, size_t itemsInIndex, size_t& estimatedItems, double& estimatedCost) { + + auto idx = indexHandle.getIndex(); + if (nullptr == idx) { + THROW_ARANGO_EXCEPTION_MESSAGE(TRI_ERROR_BAD_PARAMETER, + "The index id cannot be empty."); + } - return indexHandle.getIndex()->supportsFilterCondition( + return idx->supportsFilterCondition( condition, reference, itemsInIndex, estimatedItems, estimatedCost); } @@ -2115,8 +2121,13 @@ bool Transaction::supportsFilterCondition( std::vector> Transaction::getIndexFeatures(IndexHandle const& indexHandle, bool& isSorted, bool& isSparse) { + + auto idx = indexHandle.getIndex(); + if (nullptr == idx) { + THROW_ARANGO_EXCEPTION_MESSAGE(TRI_ERROR_BAD_PARAMETER, + "The index id cannot be empty."); + } - std::shared_ptr idx = indexHandle.getIndex(); isSorted = idx->isSorted(); isSparse = idx->sparse(); return idx->fields(); @@ -2184,7 +2195,6 @@ std::shared_ptr Transaction::indexScanForCondition( arangodb::aql::Ast* ast, arangodb::aql::AstNode const* condition, arangodb::aql::Variable const* var, uint64_t limit, uint64_t batchSize, bool reverse) { -#warning TODO Who checks if indexId is valid and is used for this collection? if (ServerState::instance()->isCoordinator()) { // The index scan is only available on DBServers and Single Server. @@ -2200,6 +2210,10 @@ std::shared_ptr Transaction::indexScanForCondition( IndexIteratorContext ctxt(_vocbase, resolver()); auto idx = indexId.getIndex(); + if (nullptr == idx) { + THROW_ARANGO_EXCEPTION_MESSAGE(TRI_ERROR_BAD_PARAMETER, + "The index id cannot be empty."); + } std::unique_ptr iterator(idx->iteratorForCondition(this, &ctxt, ast, condition, var, reverse)); @@ -2223,7 +2237,6 @@ std::shared_ptr Transaction::indexScan( std::string const& collectionName, CursorType cursorType, IndexHandle const& indexId, VPackSlice const search, uint64_t skip, uint64_t limit, uint64_t batchSize, bool reverse) { -#warning TODO Who checks if indexId is valid and is used for this collection? // For now we assume indexId is the iid part of the index. if (ServerState::instance()->isCoordinator()) { diff --git a/js/server/tests/shell/shell-transactions-noncluster.js b/js/server/tests/shell/shell-transactions-noncluster.js index 80b36cca6f..5c28e71a73 100644 --- a/js/server/tests/shell/shell-transactions-noncluster.js +++ b/js/server/tests/shell/shell-transactions-noncluster.js @@ -4144,10 +4144,7 @@ function transactionServerFailuresSuite () { //////////////////////////////////////////////////////////////////////////////// testInsertServerFailuresEmpty : function () { - var failures = [ "InsertDocumentNoLegend", - "InsertDocumentNoLegendExcept", - "InsertDocumentNoMarker", - "InsertDocumentNoMarkerExcept", + var failures = [ "InsertDocumentNoHeader", "InsertDocumentNoHeaderExcept", "InsertDocumentNoLock", @@ -4170,7 +4167,7 @@ function transactionServerFailuresSuite () { fail(); } catch (err) { - assertEqual(internal.errors.ERROR_DEBUG.code, err.errorNum); + assertEqual(internal.errors.ERROR_DEBUG.code, err.errorNum, f); } assertEqual(0, c.count()); @@ -4182,10 +4179,7 @@ function transactionServerFailuresSuite () { //////////////////////////////////////////////////////////////////////////////// testInsertServerFailuresNonEmpty : function () { - var failures = [ "InsertDocumentNoLegend", - "InsertDocumentNoLegendExcept", - "InsertDocumentNoMarker", - "InsertDocumentNoMarkerExcept", + var failures = [ "InsertDocumentNoHeader", "InsertDocumentNoHeaderExcept", "InsertDocumentNoLock", @@ -4211,7 +4205,7 @@ function transactionServerFailuresSuite () { fail(); } catch (err) { - assertEqual(internal.errors.ERROR_DEBUG.code, err.errorNum); + assertEqual(internal.errors.ERROR_DEBUG.code, err.errorNum, f); } assertEqual(1, c.count()); @@ -4224,10 +4218,7 @@ function transactionServerFailuresSuite () { //////////////////////////////////////////////////////////////////////////////// testInsertServerFailuresConstraint : function () { - var failures = [ "InsertDocumentNoLegend", - "InsertDocumentNoLegendExcept", - "InsertDocumentNoMarker", - "InsertDocumentNoMarkerExcept", + var failures = [ "InsertDocumentNoHeader", "InsertDocumentNoHeaderExcept", "InsertDocumentNoLock" ]; @@ -4247,7 +4238,7 @@ function transactionServerFailuresSuite () { fail(); } catch (err) { - assertEqual(internal.errors.ERROR_DEBUG.code, err.errorNum); + assertEqual(internal.errors.ERROR_DEBUG.code, err.errorNum, f); } assertEqual(1, c.count()); @@ -4260,10 +4251,7 @@ function transactionServerFailuresSuite () { //////////////////////////////////////////////////////////////////////////////// testInsertServerFailuresMulti : function () { - var failures = [ "InsertDocumentNoLegend", - "InsertDocumentNoLegendExcept", - "InsertDocumentNoMarker", - "InsertDocumentNoMarkerExcept", + var failures = [ "InsertDocumentNoHeader", "InsertDocumentNoHeaderExcept", "InsertDocumentNoLock", @@ -4295,10 +4283,10 @@ function transactionServerFailuresSuite () { }); } catch (err) { - assertEqual(internal.errors.ERROR_DEBUG.code, err.errorNum); + assertEqual(internal.errors.ERROR_DEBUG.code, err.errorNum, f); } - assertEqual(0, c.count()); + assertEqual(0, c.count(), f); }); }, @@ -4421,8 +4409,7 @@ function transactionServerFailuresSuite () { //////////////////////////////////////////////////////////////////////////////// testUpdateServerFailuresNonEmpty : function () { - var failures = [ "UpdateDocumentNoLegend", - "UpdateDocumentNoLegendExcept", + var failures = [ "UpdateDocumentNoMarker", "UpdateDocumentNoMarkerExcept", "UpdateDocumentNoLock", @@ -4448,7 +4435,7 @@ function transactionServerFailuresSuite () { fail(); } catch (err) { - assertEqual(internal.errors.ERROR_DEBUG.code, err.errorNum); + assertEqual(internal.errors.ERROR_DEBUG.code, err.errorNum, f); } assertEqual(1, c.count()); @@ -4462,8 +4449,7 @@ function transactionServerFailuresSuite () { //////////////////////////////////////////////////////////////////////////////// testUpdateServerFailuresMulti : function () { - var failures = [ "UpdateDocumentNoLegend", - "UpdateDocumentNoLegendExcept", + var failures = [ "UpdateDocumentNoMarker", "UpdateDocumentNoMarkerExcept", "UpdateDocumentNoLock", @@ -4500,12 +4486,12 @@ function transactionServerFailuresSuite () { }); } catch (err) { - assertEqual(internal.errors.ERROR_DEBUG.code, err.errorNum); + assertEqual(internal.errors.ERROR_DEBUG.code, err.errorNum, f); } assertEqual(10, c.count()); for (i = 0; i < 10; ++i) { - assertEqual(i, c.document("test" + i).a); + assertEqual(i, c.document("test" + i).a, f); } }); }, @@ -4515,8 +4501,7 @@ function transactionServerFailuresSuite () { //////////////////////////////////////////////////////////////////////////////// testUpdateServerFailuresMultiUpdate : function () { - var failures = [ "UpdateDocumentNoLegend", - "UpdateDocumentNoLegendExcept", + var failures = [ "UpdateDocumentNoMarker", "UpdateDocumentNoMarkerExcept", "UpdateDocumentNoLock", @@ -4558,10 +4543,10 @@ function transactionServerFailuresSuite () { assertEqual(internal.errors.ERROR_DEBUG.code, err.errorNum); } - assertEqual(10, c.count()); + assertEqual(10, c.count(), f); for (i = 0; i < 10; ++i) { - assertEqual(i, c.document("test" + i).a); - assertEqual(undefined, c.document("test" + i).b); + assertEqual(i, c.document("test" + i).a, f); + assertEqual(undefined, c.document("test" + i).b, f); } }); }, @@ -4611,8 +4596,7 @@ function transactionServerFailuresSuite () { //////////////////////////////////////////////////////////////////////////////// testMixedServerFailures : function () { - var failures = [ "UpdateDocumentNoLegend", - "UpdateDocumentNoLegendExcept", + var failures = [ "UpdateDocumentNoMarker", "UpdateDocumentNoMarkerExcept", "UpdateDocumentNoLock", @@ -4623,10 +4607,6 @@ function transactionServerFailuresSuite () { "RemoveDocumentNoLock", "RemoveDocumentNoOperation", "RemoveDocumentNoOperationExcept", - "InsertDocumentNoLegend", - "InsertDocumentNoLegendExcept", - "InsertDocumentNoMarker", - "InsertDocumentNoMarkerExcept", "InsertDocumentNoHeader", "InsertDocumentNoHeaderExcept", "InsertDocumentNoLock", @@ -4674,13 +4654,13 @@ function transactionServerFailuresSuite () { }); } catch (err) { - assertEqual(internal.errors.ERROR_DEBUG.code, err.errorNum); + assertEqual(internal.errors.ERROR_DEBUG.code, err.errorNum, f); } assertEqual(100, c.count()); for (i = 0; i < 100; ++i) { - assertEqual(i, c.document("test" + i).a); - assertEqual(undefined, c.document("test" + i).b); + assertEqual(i, c.document("test" + i).a, f); + assertEqual(undefined, c.document("test" + i).b, f); } }); }, @@ -4963,4 +4943,3 @@ jsunity.run(transactionConstraintsSuite); return jsunity.done(); - From b5a3928160bc9221117e9dc1ea745e06b8827077 Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Thu, 7 Apr 2016 17:48:58 +0200 Subject: [PATCH 20/20] fixed leak --- arangod/Indexes/SkiplistIndex.cpp | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/arangod/Indexes/SkiplistIndex.cpp b/arangod/Indexes/SkiplistIndex.cpp index a946de2f97..a0da705ce8 100644 --- a/arangod/Indexes/SkiplistIndex.cpp +++ b/arangod/Indexes/SkiplistIndex.cpp @@ -220,7 +220,12 @@ int SkiplistIndex::insert(arangodb::Transaction*, TRI_doc_mptr_t const* doc, bool) { std::vector elements; - int res = fillElement(elements, doc); + int res; + try { + res = fillElement(elements, doc); + } catch (...) { + res = TRI_ERROR_OUT_OF_MEMORY; + } if (res != TRI_ERROR_NO_ERROR) { for (auto& it : elements) { @@ -239,11 +244,6 @@ int SkiplistIndex::insert(arangodb::Transaction*, TRI_doc_mptr_t const* doc, for (size_t i = 0; i < count; ++i) { res = _skiplistIndex->insert(elements[i]); - if (res == TRI_ERROR_ARANGO_UNIQUE_CONSTRAINT_VIOLATED && !_unique) { - // We ignore unique_constraint violated if we are not unique - res = TRI_ERROR_NO_ERROR; - } - if (res != TRI_ERROR_NO_ERROR) { TRI_index_element_t::freeElement(elements[i]); // Note: this element is freed already @@ -254,7 +254,11 @@ int SkiplistIndex::insert(arangodb::Transaction*, TRI_doc_mptr_t const* doc, _skiplistIndex->remove(elements[j]); // No need to free elements[j] skiplist has taken over already } - + + if (res == TRI_ERROR_ARANGO_UNIQUE_CONSTRAINT_VIOLATED && !_unique) { + // We ignore unique_constraint violated if we are not unique + res = TRI_ERROR_NO_ERROR; + } break; } } @@ -269,7 +273,12 @@ int SkiplistIndex::remove(arangodb::Transaction*, TRI_doc_mptr_t const* doc, bool) { std::vector elements; - int res = fillElement(elements, doc); + int res; + try { + res = fillElement(elements, doc); + } catch (...) { + res = TRI_ERROR_OUT_OF_MEMORY; + } if (res != TRI_ERROR_NO_ERROR) { for (auto& it : elements) {