yakaz / elasticsearch-action-updatebyquery

ElasticSearch Update By Query action plugin
113 stars 24 forks source link

Problem Elasticsearch 1.6 #38

Closed limegup closed 9 years ago

limegup commented 9 years ago

Hi,

I tried to run:

POST /myindex/mydoc/_update_by_query { "query" : { "match_all": {} }, "script" : "ctx._source.somevalue = 100" }

I got:

{ "error": "NoSuchMethodError[org.elasticsearch.rest.RestRequest.contentUnsafe()Z]", "status": 500 }

mesirendon commented 9 years ago

+1 I've created the tweeter index to test and doing as the README says, I get the same error

comdiv commented 9 years ago

+1

comdiv commented 9 years ago

I have "fix" it by simple remove all things that uses contentUnsafe() - it works now on 1.6 but i think it's not much good....

igor-kupczynski commented 9 years ago

+1 as I see the same on my dev box

NoSuchMethodError[org.elasticsearch.rest.RestRequest.contentUnsafe()Z]

@comdiv so this ctx._source.somevalue = 100 uses contentUnsafe()? How we can replace it with something else?

I see this ticket in ES itself https://github.com/elastic/elasticsearch-transport-memcached/issues/40 and the commit https://github.com/elastic/elasticsearch-transport-memcached/commit/32415cc397fc212b4dbbd6115c03832c56c29022 but there is no info on what we can use instead

comdiv commented 9 years ago

That is patch

diff --git a/src/main/java/org/elasticsearch/action/updatebyquery/ShardUpdateByQueryRequest.java b/src/main/java/org/elasticsearch/action/updatebyquery/ShardUpdateByQueryRequest.java
index 7d6718d..a4288c5 100644
--- a/src/main/java/org/elasticsearch/action/updatebyquery/ShardUpdateByQueryRequest.java
+++ b/src/main/java/org/elasticsearch/action/updatebyquery/ShardUpdateByQueryRequest.java
@@ -97,12 +97,6 @@ public class ShardUpdateByQueryRequest extends ShardReplicationOperationRequest
         return bulkResponseOption;
     }

-    @Override
-    public void beforeLocalFork() {
-        if (sourceUnsafe) {
-            source = source.copyBytesArray();
-        }
-    }

     @Override
     public ActionRequestValidationException validate() {
diff --git a/src/main/java/org/elasticsearch/action/updatebyquery/TransportShardUpdateByQueryAction.java b/src/main/java/org/elasticsearch/action/updatebyquery/TransportShardUpdateByQueryAction.java
index 577ca93..fd42403 100644
--- a/src/main/java/org/elasticsearch/action/updatebyquery/TransportShardUpdateByQueryAction.java
+++ b/src/main/java/org/elasticsearch/action/updatebyquery/TransportShardUpdateByQueryAction.java
@@ -114,7 +114,6 @@ public class TransportShardUpdateByQueryAction extends TransportAction<ShardUpda
         }

         if (request.operationThreaded()) {
-            request.beforeLocalFork();
             threadPool.executor(ThreadPool.Names.BULK).execute(new Runnable() {

                 public void run() {
diff --git a/src/main/java/org/elasticsearch/rest/action/updatebyquery/RestUpdateByQueryAction.java b/src/main/java/org/elasticsearch/rest/action/updatebyquery/RestUpdateByQueryAction.java
index 1ddf630..55e43e1 100644
--- a/src/main/java/org/elasticsearch/rest/action/updatebyquery/RestUpdateByQueryAction.java
+++ b/src/main/java/org/elasticsearch/rest/action/updatebyquery/RestUpdateByQueryAction.java
@@ -87,7 +87,7 @@ public class RestUpdateByQueryAction extends BaseRestHandler {

         // see if we have it in the body
         if (request.hasContent()) {
-            udqRequest.source(request.content(), request.contentUnsafe());
+            udqRequest.source(request.content(), false);
         } else if (request.hasParam("source")) {
             udqRequest.source(new BytesArray(request.param("source")), false);
         } else if (request.hasParam("q")) {

And it works - maybe it work not "safe" but all groovy scripts are executed well

igor-kupczynski commented 9 years ago

OK, thanks, I'll try with my tests and let you know

igor-kupczynski commented 9 years ago

@comdiv @apeteri Thanks guys after applying pull request on my repo it works great.

We run some automated and manual tests against a dev cluster and we see no issues with update_by_query. I hope this pull request will get merged quickly.

redserpent7 commented 9 years ago

Any updates on this? When should we expect the fix to be merged? ES is on 1.7.1 now

ofavre commented 9 years ago

v2.6.0, supporting ES 1.6.x is not released and should be available within 2-4 hours.

ofavre commented 9 years ago

v2.6.0 supports ES 1.7.1 as well.