bug-fix/internal-issue-#802 (arangodb#13797)

* allow lazy index snapshot instantiation for secondary transactions * fix jslint errors
flyarong · Mar 24, 2021 · fa4b28f · fa4b28f
1 parent 03b7384
commit fa4b28f
Show file tree

Hide file tree

Showing 4 changed files with 52 additions and 16 deletions.
diff --git a/CHANGELOG b/CHANGELOG
@@ -1,6 +1,8 @@
 devel
 -----
 
+* Fix implicit capture of views in a context of JS transaction.
+
 * Fix a crash caused by returning a result produced by ANALYZER function.
 
 * Update the Web UI's list of built-in AQL functions for proper syntax
@@ -20,7 +22,7 @@ devel
 * Fix an assertion failure that occurred when restoring view definitions from
   a cluster into a single server.
 
-* Added new ArangoSearch analyzer type "mask".
+* Added new ArangoSearch analyzer type "stopwords".
 
 * Fix error message in case of index unique constraint violations. They were
   lacking the actual error message (i.e. "unique constraint violated") and

diff --git a/arangod/Aql/IResearchViewNode.cpp b/arangod/Aql/IResearchViewNode.cpp
@@ -705,9 +705,6 @@ typedef std::shared_ptr<IResearchView::Snapshot const> SnapshotPtr;
 SnapshotPtr snapshotDBServer(IResearchViewNode const& node, transaction::Methods& trx) {
   TRI_ASSERT(ServerState::instance()->isDBServer());
 
-  static IResearchView::SnapshotMode const SNAPSHOT[]{IResearchView::SnapshotMode::FindOrCreate,
-                                                      IResearchView::SnapshotMode::SyncAndReplace};
-
   auto& view = LogicalView::cast<IResearchView>(*node.view());
   auto& options = node.options();
   auto* resolver = trx.resolver();
@@ -738,9 +735,12 @@ SnapshotPtr snapshotDBServer(IResearchViewNode const& node, transaction::Methods
     snapshotKey = &node;
   }
 
+  IResearchView::SnapshotMode const mode = !options.forceSync
+    ? IResearchView::SnapshotMode::FindOrCreate
+    : IResearchView::SnapshotMode::SyncAndReplace;
+
   // use aliasing ctor
-  return {SnapshotPtr(), view.snapshot(trx, SNAPSHOT[size_t(options.forceSync)],
-                                       &collections, snapshotKey)};
+  return {SnapshotPtr(), view.snapshot(trx, mode, &collections, snapshotKey)};
 }
 
 /// @brief Since single-server is transactional we do the following:
@@ -758,15 +758,19 @@ SnapshotPtr snapshotDBServer(IResearchViewNode const& node, transaction::Methods
 SnapshotPtr snapshotSingleServer(IResearchViewNode const& node, transaction::Methods& trx) {
   TRI_ASSERT(ServerState::instance()->isSingleServer());
 
-  static IResearchView::SnapshotMode const SNAPSHOT[]{IResearchView::SnapshotMode::Find,
-                                                      IResearchView::SnapshotMode::SyncAndReplace};
-
   auto& view = LogicalView::cast<IResearchView>(*node.view());
   auto& options = node.options();
 
+  IResearchView::SnapshotMode mode = IResearchView::SnapshotMode::Find;
+
+  if (options.forceSync) {
+    mode = IResearchView::SnapshotMode::SyncAndReplace;
+  } else if (!trx.isMainTransaction()) {
+    mode = IResearchView::SnapshotMode::FindOrCreate;
+  }
+
   // use aliasing ctor
-  auto reader = SnapshotPtr(SnapshotPtr(),
-                            view.snapshot(trx, SNAPSHOT[size_t(options.forceSync)]));
+  auto reader = SnapshotPtr(SnapshotPtr(), view.snapshot(trx, mode));
 
   if (options.restrictSources && reader) {
     // reassemble reader

diff --git a/tests/js/common/aql/aql-view-arangosearch-cluster.inc b/tests/js/common/aql/aql-view-arangosearch-cluster.inc
@@ -253,6 +253,22 @@
         assertEqual(result[0], "the quick brown fox jumps over the lazy dog");
       },
 
+      testTransactionRegistration : function () {
+        // ensure data is synced
+        db._query("FOR d IN CompoundView OPTIONS {waitForSync:true} LIMIT 1 RETURN 1");
+
+        // implicit read lock
+        var result = db._executeTransaction({
+          collections: { },
+          action: function () {
+            var db = require("@arangodb").db;
+            return db._query("FOR d IN CompoundView SEARCH d.name == 'full' RETURN d.text").toArray();
+          }
+        });
+        assertEqual("the quick brown fox jumps over the lazy dog", result[0]);
+        assertEqual(1, result.length);
+      },
+
       testViewCollectionOptions : function() {
         var result = db._query("FOR doc IN CompoundView SEARCH doc.a == 'foo' OPTIONS { waitForSync: true, collections : [ 'UnitTestsCollection' ] } RETURN doc").toArray();
 

diff --git a/tests/js/common/aql/aql-view-arangosearch-noncluster.js b/tests/js/common/aql/aql-view-arangosearch-noncluster.js
@@ -88,29 +88,29 @@ function iResearchAqlTestSuite () {
         }
       };
       arrayV.properties(meta);
-      
+
       db._drop("TestsCollectionWithManyFields");
       let mfc = db._create("TestsCollectionWithManyFields");
       mfc.save({field1:"1value", field2:"2value", field3: 1, field4: 11111, field5: 1, field6: 1});
       mfc.save({field1:"1value1", field2:"2value1", field3: 2, field4: 11112, field5: 2, field6: 2});
       mfc.save({field1:"1value2", field2:"2value2", field3: 3, field4: 11113, field5: 3, field6: 3});
       mfc.save({field1:"1value3", field2:"2value3", field3: 4, field4: 11114, field5: 4, field6: 4});
-      
+
       try { analyzers.remove("customAnalyzer", true); } catch(err) {}
       analyzers.save("customAnalyzer", "text",  {"locale": "en.utf-8",
                                                  "case": "lower",
                                                  "stopwords": [],
                                                  "accent": false,
                                                  "stemming": false},
                                                  ["position", "norm", "frequency"]);
-                                                 
+
       let wps = db._createView("WithPrimarySort", "arangosearch", 
                                {primarySort: [{field: "field1", direction: "asc"},
                                               {field: "field2", direction: "asc"},
                                               {field: "field3", direction: "asc"},
                                               {field: "field4", direction: "asc"},
                                               {field: "_key", direction: "asc"}]});
-                                    
+
       wps.properties({links:{TestsCollectionWithManyFields: {
                               storeValues: "id",
                               analyzers: ["customAnalyzer"],
@@ -314,8 +314,22 @@ function iResearchAqlTestSuite () {
     },
 
     testTransactionRegistration : function () {
-      // read lock
+      // ensure data is synced
+      db._query("FOR d IN CompoundView OPTIONS {waitForSync:true} LIMIT 1 RETURN 1");
+
+      // implicit read lock
       var result = db._executeTransaction({
+        collections: { },
+        action: function () {
+          var db = require("@arangodb").db;
+          return db._query("FOR d IN CompoundView SEARCH d.name == 'full' RETURN d.text").toArray();
+        }
+      });
+      assertEqual("the quick brown fox jumps over the lazy dog", result[0]);
+      assertEqual(1, result.length);
+
+      // read lock
+      result = db._executeTransaction({
         collections: {
           allowImplicit: false,
           read: [ v.name() ]