From c8ddff2a9ba6d2eb82f47ceb52be5d4f19faf718 Mon Sep 17 00:00:00 2001 From: Frank Celler Date: Fri, 30 May 2014 11:39:11 +0200 Subject: [PATCH 1/5] fixed shutdown for busy V8 contexts --- arangod/RestServer/ArangoServer.cpp | 18 ++++++++-- arangod/RestServer/ConsoleThread.cpp | 10 +++--- arangod/RestServer/ConsoleThread.h | 1 - arangod/V8Server/ApplicationV8.cpp | 42 ++++++++++++++++++++++-- arangod/VocBase/datafile.cpp | 4 +-- lib/Dispatcher/ApplicationDispatcher.cpp | 12 +++++-- lib/Dispatcher/ApplicationDispatcher.h | 6 ++++ 7 files changed, 77 insertions(+), 16 deletions(-) diff --git a/arangod/RestServer/ArangoServer.cpp b/arangod/RestServer/ArangoServer.cpp index 2414809321..d8f3019699 100644 --- a/arangod/RestServer/ArangoServer.cpp +++ b/arangod/RestServer/ArangoServer.cpp @@ -761,6 +761,7 @@ void ArangoServer::buildApplicationServer () { //////////////////////////////////////////////////////////////////////////////// int ArangoServer::startupServer () { + OperationMode::server_operation_mode_e mode = OperationMode::determineMode(_applicationServer->programOptions()); // ............................................................................. // prepare the various parts of the Arango server @@ -783,8 +784,21 @@ int ArangoServer::startupServer () { assert(vocbase != 0); // initialise V8 + size_t concurrency = _dispatcherThreads; + + if (mode == OperationMode::MODE_CONSOLE) { + // one V8 instance is taken by the console + ++concurrency; + } + else if (mode == OperationMode::MODE_UNITTESTS || mode == OperationMode::MODE_SCRIPT) { + if (concurrency == 1) { + // at least two to allow the test-runner and the scheduler to use a V8 + concurrency = 2; + } + } + _applicationV8->setVocbase(vocbase); - _applicationV8->setConcurrency(_dispatcherThreads); + _applicationV8->setConcurrency(concurrency); if (_applicationServer->programOptions().has("upgrade")) { _applicationV8->performUpgrade(); @@ -855,8 +869,6 @@ int ArangoServer::startupServer () { LOG_INFO("ArangoDB (version " TRI_VERSION_FULL ") is ready for business. Have fun!"); - OperationMode::server_operation_mode_e mode = OperationMode::determineMode(_applicationServer->programOptions()); - int res; if (mode == OperationMode::MODE_CONSOLE) { diff --git a/arangod/RestServer/ConsoleThread.cpp b/arangod/RestServer/ConsoleThread.cpp index 39af6ad6da..453ff464b1 100644 --- a/arangod/RestServer/ConsoleThread.cpp +++ b/arangod/RestServer/ConsoleThread.cpp @@ -90,19 +90,20 @@ void ConsoleThread::run () { try { inner(); - _applicationV8->exitContext(_context); - _done = 1; } catch (const char*) { - _applicationV8->exitContext(_context); - _done = 1; } catch (...) { _applicationV8->exitContext(_context); _done = 1; + _applicationServer->beginShutdown(); throw; } + + _applicationV8->exitContext(_context); + _done = 1; + _applicationServer->beginShutdown(); } // ----------------------------------------------------------------------------- @@ -156,7 +157,6 @@ void ConsoleThread::inner () { if (input == 0) { _userAborted = true; - _applicationServer->beginShutdown(); // this will be caught by "run" throw "user aborted"; diff --git a/arangod/RestServer/ConsoleThread.h b/arangod/RestServer/ConsoleThread.h index 4cfe8bce1c..1e20b832de 100644 --- a/arangod/RestServer/ConsoleThread.h +++ b/arangod/RestServer/ConsoleThread.h @@ -96,7 +96,6 @@ namespace triagens { void userAbort () { _userAborted = true; - TRI_CLOSE(0); } //////////////////////////////////////////////////////////////////////////////// diff --git a/arangod/V8Server/ApplicationV8.cpp b/arangod/V8Server/ApplicationV8.cpp index 477ca7d34d..d6a9327b3b 100644 --- a/arangod/V8Server/ApplicationV8.cpp +++ b/arangod/V8Server/ApplicationV8.cpp @@ -746,6 +746,25 @@ bool ApplicationV8::start () { void ApplicationV8::close () { _stopping = 1; _contextCondition.broadcast(); + + // unregister all tasks + if (_scheduler != nullptr) { + _scheduler->scheduler()->unregisterUserTasks(); + } + + // wait for all contexts to finish + for (size_t n = 0; n < 10 * 5; ++n) { + CONDITION_LOCKER(guard, _contextCondition); + + if (_busyContexts.empty()) { + LOG_DEBUG("no busy V8 contexts"); + break; + } + + LOG_DEBUG("waiting for %d busy V8 contexts to finish", (int) _busyContexts.size()); + + guard.wait(100000); + } } //////////////////////////////////////////////////////////////////////////////// @@ -753,9 +772,26 @@ void ApplicationV8::close () { //////////////////////////////////////////////////////////////////////////////// void ApplicationV8::stop () { - // unregister all tasks - if (_scheduler != nullptr) { - _scheduler->scheduler()->unregisterUserTasks(); + + //send all busy contexts a termate signal + { + CONDITION_LOCKER(guard, _contextCondition); + + for (auto it = _busyContexts.begin(); it != _busyContexts.end(); ++it) { + LOG_WARNING("sending termination signal to V8 context"); + v8::V8::TerminateExecution((*it)->_isolate); + } + } + + // wait for one minute + for (size_t n = 0; n < 10 * 60; ++n) { + CONDITION_LOCKER(guard, _contextCondition); + + if (_busyContexts.empty()) { + break; + } + + guard.wait(100000); } // stop GC diff --git a/arangod/VocBase/datafile.cpp b/arangod/VocBase/datafile.cpp index 3fda1abbcb..59bbc1c088 100644 --- a/arangod/VocBase/datafile.cpp +++ b/arangod/VocBase/datafile.cpp @@ -114,7 +114,7 @@ static bool SyncDatafile (const TRI_datafile_t* const datafile, return true; } - assert(datafile->_fd > 0); + assert(datafile->_fd >= 0); if (begin == end) { // no need to sync @@ -212,7 +212,7 @@ static void InitDatafile (TRI_datafile_t* datafile, assert(fd == -1); } else { - assert(fd > 0); + assert(fd >= 0); } datafile->_state = TRI_DF_STATE_READ; diff --git a/lib/Dispatcher/ApplicationDispatcher.cpp b/lib/Dispatcher/ApplicationDispatcher.cpp index 416f8e9369..5f695b9de8 100644 --- a/lib/Dispatcher/ApplicationDispatcher.cpp +++ b/lib/Dispatcher/ApplicationDispatcher.cpp @@ -214,6 +214,16 @@ bool ApplicationDispatcher::open () { /// {@inheritDoc} //////////////////////////////////////////////////////////////////////////////// +void ApplicationDispatcher::close () { + if (_dispatcher != 0) { + _dispatcher->beginShutdown(); + } +} + +//////////////////////////////////////////////////////////////////////////////// +/// {@inheritDoc} +//////////////////////////////////////////////////////////////////////////////// + void ApplicationDispatcher::stop () { if (_dispatcherReporterTask != 0) { _dispatcherReporterTask = 0; @@ -222,8 +232,6 @@ void ApplicationDispatcher::stop () { if (_dispatcher != 0) { static size_t const MAX_TRIES = 50; // 10 seconds (50 * 200 ms) - _dispatcher->beginShutdown(); - for (size_t count = 0; count < MAX_TRIES && _dispatcher->isRunning(); ++count) { LOG_TRACE("waiting for dispatcher to stop"); usleep(200 * 1000); diff --git a/lib/Dispatcher/ApplicationDispatcher.h b/lib/Dispatcher/ApplicationDispatcher.h index f6b0a8b532..84c30fe561 100644 --- a/lib/Dispatcher/ApplicationDispatcher.h +++ b/lib/Dispatcher/ApplicationDispatcher.h @@ -136,6 +136,12 @@ namespace triagens { bool open (); +//////////////////////////////////////////////////////////////////////////////// +/// {@inheritDoc} +//////////////////////////////////////////////////////////////////////////////// + + void close (); + //////////////////////////////////////////////////////////////////////////////// /// {@inheritDoc} //////////////////////////////////////////////////////////////////////////////// From c2478e30ecde49141c4827fca9e2baf04c09d783 Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Fri, 30 May 2014 12:09:17 +0200 Subject: [PATCH 2/5] adjusted types for Windows --- lib/BasicsC/operating-system.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/BasicsC/operating-system.h b/lib/BasicsC/operating-system.h index 5dc053d72b..2dc537e1f2 100644 --- a/lib/BasicsC/operating-system.h +++ b/lib/BasicsC/operating-system.h @@ -674,7 +674,7 @@ typedef unsigned char bool; #define TRI_CLOSE _close #define TRI_CREATE(a,b,c) TRI_createFile((a), (b), (c)) #define TRI_GETCWD _getcwd -#define TRI_LSEEK _lseek +#define TRI_LSEEK _lseeki64 #define TRI_MKDIR(a,b) _mkdir((a)) #define TRI_OPEN(a,b) TRI_OPEN_WIN32((a), (b)) #define TRI_READ _read @@ -682,9 +682,9 @@ typedef unsigned char bool; #define TRI_UNLINK _unlink #define TRI_WRITE _write -#define TRI_write_t unsigned long -#define TRI_read_t unsigned long -#define TRI_lseek_t long +#define TRI_write_t unsigned int +#define TRI_read_t unsigned int +#define TRI_lseek_t __int64 #define TRI_LAST_ERROR_STR strerror(errno) From 39afcff925e64cbf93c2984f8b2148cc96741361 Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Fri, 30 May 2014 12:27:13 +0200 Subject: [PATCH 3/5] type casts --- arangod/VocBase/datafile.cpp | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/arangod/VocBase/datafile.cpp b/arangod/VocBase/datafile.cpp index 59bbc1c088..7732240db1 100644 --- a/arangod/VocBase/datafile.cpp +++ b/arangod/VocBase/datafile.cpp @@ -146,7 +146,7 @@ static int TruncateDatafile (TRI_datafile_t* const datafile, const off_t length) static int CreateSparseFile (char const* filename, const TRI_voc_size_t maximalSize) { - off_t offset; + TRI_lseek_t offset; char zero; ssize_t res; int fd; @@ -162,9 +162,9 @@ static int CreateSparseFile (char const* filename, } // create sparse file - offset = TRI_LSEEK(fd, maximalSize - 1, SEEK_SET); + offset = TRI_LSEEK(fd, (TRI_lseek_t) (maximalSize - 1), SEEK_SET); - if (offset == (off_t) -1) { + if (offset == (TRI_lseek_t) -1) { TRI_set_errno(TRI_ERROR_SYS_ERROR); TRI_CLOSE(fd); @@ -192,7 +192,6 @@ static int CreateSparseFile (char const* filename, return fd; } - //////////////////////////////////////////////////////////////////////////////// /// @brief initialises a datafile //////////////////////////////////////////////////////////////////////////////// @@ -266,7 +265,6 @@ static int TruncateAndSealDatafile (TRI_datafile_t* datafile, int fd; int res; size_t maximalSize; - off_t offset; void* data; void* mmHandle; @@ -295,9 +293,9 @@ static int TruncateAndSealDatafile (TRI_datafile_t* datafile, } // create sparse file - offset = TRI_LSEEK(fd, (TRI_lseek_t) maximalSize - 1, SEEK_SET); + TRI_lseek_t offset = TRI_LSEEK(fd, (TRI_lseek_t) (maximalSize - 1), SEEK_SET); - if (offset == (off_t) -1) { + if (offset == (TRI_lseek_t) -1) { TRI_set_errno(TRI_ERROR_SYS_ERROR); TRI_CLOSE(fd); From 0e326149d89f48263140dc51f478b6d4e645dfb6 Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Fri, 30 May 2014 12:53:14 +0200 Subject: [PATCH 4/5] added test case --- UnitTests/Makefile.unittests | 1 + 1 file changed, 1 insertion(+) diff --git a/UnitTests/Makefile.unittests b/UnitTests/Makefile.unittests index 6e19844281..6e8719744e 100755 --- a/UnitTests/Makefile.unittests +++ b/UnitTests/Makefile.unittests @@ -855,6 +855,7 @@ unittests-arangob: @builddir@/bin/arangob --configuration none --quiet --server.username "$(USERNAME)" --server.password "$(PASSWORD)" --server.endpoint unix://$(VOCDIR)/arango.sock --requests 500 --concurrency 3 --test aqltrx --complexity 1 || test "x$(FORCE)" == "x1" @builddir@/bin/arangob --configuration none --quiet --server.username "$(USERNAME)" --server.password "$(PASSWORD)" --server.endpoint unix://$(VOCDIR)/arango.sock --requests 100 --concurrency 3 --test counttrx || test "x$(FORCE)" == "x1" @builddir@/bin/arangob --configuration none --quiet --server.username "$(USERNAME)" --server.password "$(PASSWORD)" --server.endpoint unix://$(VOCDIR)/arango.sock --requests 500 --concurrency 3 --test multitrx || test "x$(FORCE)" == "x1" + @builddir@/bin/arangob --configuration none --quiet --server.username "$(USERNAME)" --server.password "$(PASSWORD)" --server.endpoint unix://$(VOCDIR)/arango.sock --requests 500 --concurrency 3 --test import-document --complexity 500 || test "x$(FORCE)" == "x1" kill `cat $(PIDFILE)` From 982f820d9994ff5ddbde9c173b8bca5c83c50c1c Mon Sep 17 00:00:00 2001 From: Heiko Kernbach Date: Fri, 30 May 2014 16:16:41 +0200 Subject: [PATCH 5/5] added tests for collectionsItemView --- .../specs/views/collectionsItemViewSpec.js | 179 ++++++++++++++++++ 1 file changed, 179 insertions(+) diff --git a/js/apps/system/aardvark/test/specs/views/collectionsItemViewSpec.js b/js/apps/system/aardvark/test/specs/views/collectionsItemViewSpec.js index e75543fa96..7c70d3b213 100644 --- a/js/apps/system/aardvark/test/specs/views/collectionsItemViewSpec.js +++ b/js/apps/system/aardvark/test/specs/views/collectionsItemViewSpec.js @@ -133,7 +133,186 @@ expect(window.modalView.show).toHaveBeenCalled(); }); + it("should stop an events propagination", function() { + var e = { + stopPropagation: function(){} + }; + spyOn(e, "stopPropagation"); + tile1.noop(e); + expect(e.stopPropagation).toHaveBeenCalled(); + }); + + it("should load a collection", function() { + spyOn(tile1.model, "loadCollection"); + spyOn(tile1, "render"); + spyOn(window.modalView, "hide"); + tile1.loadCollection(); + expect(tile1.model.loadCollection).toHaveBeenCalled(); + expect(tile1.render).toHaveBeenCalled(); + expect(window.modalView.hide).toHaveBeenCalled(); + }); + + it("should unload a collection", function() { + spyOn(tile1.model, "unloadCollection"); + spyOn(tile1, "render"); + spyOn(window.modalView, "hide"); + tile1.unloadCollection(); + expect(tile1.model.unloadCollection).toHaveBeenCalled(); + expect(tile1.render).toHaveBeenCalled(); + expect(window.modalView.hide).toHaveBeenCalled(); + }); + + it("should delete a collection with success", function() { + spyOn(tile1.model, "destroy"); + spyOn(tile1.collectionsView, "render"); + spyOn(window.modalView, "hide"); + tile1.deleteCollection(); + expect(tile1.model.destroy).toHaveBeenCalled(); + expect(tile1.collectionsView.render).toHaveBeenCalled(); + expect(window.modalView.hide).toHaveBeenCalled(); + }); + + it("should save a modified collection (unloaded collection, save error)", function() { + window.App = { + notificationList: { + add: function() { + } + } + }; + + spyOn(arangoHelper, "arangoError"); + spyOn(tile1.model, "renameCollection"); + + tile1.saveModifiedCollection(); + + expect(tile1.model.renameCollection).toHaveBeenCalled(); + expect(arangoHelper.arangoError).toHaveBeenCalled(); + }); + + it("should save a modified collection (loaded collection, save error)", function() { + tile1.model.set('status', "loaded"); + window.App = { + notificationList: { + add: function() { + } + } + }; + + spyOn(arangoHelper, "arangoError"); + spyOn(tile1.model, "renameCollection"); + + tile1.saveModifiedCollection(); + + expect(tile1.model.renameCollection).toHaveBeenCalled(); + expect(arangoHelper.arangoError).toHaveBeenCalled(); + }); }); + it("should save a modified collection (unloaded collection, success)", function() { + window.App = { + notificationList: { + add: function() { + } + } + }; + + spyOn(tile1.model, "renameCollection").andReturn(true); + spyOn(tile1.collectionsView, "render"); + spyOn(window.modalView, "hide"); + tile1.saveModifiedCollection(); + + expect(window.modalView.hide).toHaveBeenCalled(); + expect(tile1.collectionsView.render).toHaveBeenCalled(); + expect(tile1.model.renameCollection).toHaveBeenCalled(); + }); + + it("should save a modified collection (loaded collection, success)", function() { + tile1.model.set('status', "loaded"); + window.App = { + notificationList: { + add: function() { + } + } + }; + + var tempdiv = document.createElement("div"); + tempdiv.id = "change-collection-size"; + document.body.appendChild(tempdiv); + $('#change-collection-size').val(123123123123); + + spyOn(tile1.model, "changeCollection").andReturn(true); + spyOn(tile1.model, "renameCollection").andReturn(true); + spyOn(tile1.collectionsView, "render"); + spyOn(window.modalView, "hide"); + tile1.saveModifiedCollection(); + + expect(window.modalView.hide).toHaveBeenCalled(); + expect(tile1.collectionsView.render).toHaveBeenCalled(); + expect(tile1.model.renameCollection).toHaveBeenCalled(); + + document.body.removeChild(tempdiv); + }); + + it("should not save a modified collection (invalid data, result)", function() { + tile1.model.set('status', "loaded"); + window.App = { + notificationList: { + add: function() { + } + } + }; + + var tempdiv = document.createElement("div"); + tempdiv.id = "change-collection-size"; + document.body.appendChild(tempdiv); + $('#change-collection-size').val(123123123123); + + spyOn(arangoHelper, "arangoError"); + spyOn(arangoHelper, "arangoNotification"); + spyOn(tile1.model, "changeCollection").andReturn(false); + spyOn(tile1.model, "renameCollection").andReturn(false); + spyOn(tile1.collectionsView, "render"); + spyOn(window.modalView, "hide"); + tile1.saveModifiedCollection(); + + expect(window.modalView.hide).not.toHaveBeenCalled(); + expect(tile1.collectionsView.render).not.toHaveBeenCalled(); + expect(tile1.model.renameCollection).toHaveBeenCalled(); + expect(arangoHelper.arangoError).toHaveBeenCalled(); + + document.body.removeChild(tempdiv); + }); + + it("should not save a modified collection (invalid data, result)", function() { + tile1.model.set('status', "loaded"); + window.App = { + notificationList: { + add: function() { + } + } + }; + + var tempdiv = document.createElement("div"); + tempdiv.id = "change-collection-size"; + document.body.appendChild(tempdiv); + $('#change-collection-size').val(123123123123); + + spyOn(arangoHelper, "arangoError"); + spyOn(arangoHelper, "arangoNotification"); + spyOn(tile1.model, "changeCollection").andReturn(false); + spyOn(tile1.model, "renameCollection").andReturn(true); + spyOn(tile1.collectionsView, "render"); + spyOn(window.modalView, "hide"); + tile1.saveModifiedCollection(); + + expect(window.modalView.hide).not.toHaveBeenCalled(); + expect(tile1.collectionsView.render).not.toHaveBeenCalled(); + expect(tile1.model.renameCollection).toHaveBeenCalled(); + expect(arangoHelper.arangoNotification).toHaveBeenCalled(); + + document.body.removeChild(tempdiv); + }); + + }); }());