What concerns me about this pull request is that it adds to the REST API sometimes that is an implementation detail of updates.

Can you give more content about your use-case, maybe it could be fixed in a different way?

I just realised I didn't mention the issue where we talked about this #9642

In short, we get a huge amount of updates coming in that are noop. It's out of our hands, so when we changed to the update api with noop we dropped from ~60 indexes per second to <10. Now we want to renormalise data even more (across two types) and that would mean a scroll and scan with bulk update when the data changes. Since we don't want to do this on a noop, it'd be great to know when an update is a noop.

@nik9000 Here's a starter implementation for noop responses re: #9642

I imagine I'll need to do some doc updates along with it correct? I thought I'd just start with the response implementation to make sure i'm on the right track.

This makes sense to me. If we go ahead and get it in, we need to serialize the new noop field (readFrom and writeTo methods). Tests are needed too, as well as updated docs ;)

@snikch are you interested in providing the extra bits that @javanna asked for here: #9736 (comment)

@clintongormley Yup, I can get that sorted. Sorry, work's been full on - will have time over the next week or two. Cheers.

What version is this expected to be released?

@jpountz I've updated this PR as per @javanna's comments. Let me know if there's anything you'd like changed / added.

Is this going to be merged for next release? thanks!

I suspect it'd be good to have a test in the Java api too.

Sorry I didn't get those pings back in February. I'm going to go and tweak my gmail rules....

Also, just quickly scanning the diff - I suspect this might work for scripted updates as well. It'd be cool to add a test for that. Even if the test shows it doesn't work for scripted updates it'd be useful.

Sorry I didn't get those pings back in February. I'm going to go and tweak my gmail rules....

And it turns out there isn't a way to add a special label to emails in which I'm pinged.... sadface

Thanks Nik - I'll have a look at adding tests as per your comments. Is the version checking you mention the elasticsearch version (so support compatibility between clients which don't send the same data I guess?). If so (or not) can you point me in the direction of an example in the codebase currently?

@snikch have a look at NodesHotThreadsRequest - it does the version checking on read and write. The idea is that when you have two versions of Elasticsearch running in the cluster you don't want them to not understand eachother - both for clients and during a rolling upgrade.

I'd target this at 2.0 only, so no need for the version checks.

Cool thanks @clintongormley. So just Unit tests required.

In light of #11684 it might be nice to generalise this response a bit, so instead of just noop: true/false, perhaps:

"operation": "create|index|delete|noop"

Hi everyone, any hint about the ES version that will contain this feature?

Hi everyone, any hint about the ES version that will contain this feature?

Given the conversation above - 2.0 or later most probably.

@snikch Are you interested in picking this up again?

Yup, sure. Somehow I missed the whole conversation about opcodes.

Hiya @snikch

Just a ping to remind you about this :)

Thanks @clintongormley. I started on this and ran into an issue. Previously we were binary "noop": true/false, which was easy to determine since a no-op update is the only information we needed to know (everything else is not a no-op). I managed to get create|index|noop all working, but my Java spelunking skills came apart when trying to figure out exactly how the call chain worked in a delete case. I think I've tried all the entry points I could find, but just cannot work out how the action/update/UpdateResponse object gets instantiated.

Some guidance would be much appreciated from anybody who can help. I've just pushed up my work in progress.

thanks @snikch - perhaps @nik9000 could provide some guidance?

thanks @snikch - perhaps @nik9000 could provide some guidance?

Yeah! I'll add this to my list of things to do today. I'm going to disney world tomorrow for a week so if I miss it today I'll probably not reply for a while.

Ok @snikch! I'd start this by making the following change:

diff --git a/core/src/main/java/org/elasticsearch/action/DocWriteResponse.java b/core/src/main/java/org/elasticsearch/action/DocWriteResponse.java
index c0389c6..6c28ed3 100644
--- a/core/src/main/java/org/elasticsearch/action/DocWriteResponse.java
+++ b/core/src/main/java/org/elasticsearch/action/DocWriteResponse.java
+    public abstract Operation getOperation();
+
+    public static enum Operation {
+        CREATE(0),
+        INDEX(1),
+        DELETE(2),
+        NOOP(3);
+
+        private final byte id;
+
+        private Operation(int id) {
+            this.id = (byte)id;
+        }
+
+        public byte getId() {
+            return id;
+        }
+
+        public static final Operation formId(byte id) {
+            switch(id) {
+            case 0:
+                return CREATE;
+            case 1:
+                return INDEX;
+            case 2:
+                return DELETE;
+            case 3:
+                return NOOP;
+            default:
+                throw new IllegalArgumentException("Unknown Operation id: " + id);
+            }
+        }
+    }

And then playing whack-a-mole with errors for a while. That is a real nice way to do java spelunking because my IDE just tells me what to do next and all those changes are pretty mechanical.

Next I'd add:

            .field("_operation", getOperation().toString());

to DocWriteResponse#toXContent.

You'd need to add an operation field to UpdateResponse because it can contain any operation but the other responses can just use the data we have. I wouldn't worry about backwards compatibility here though. I'd just target master so it'll be in 3.0.0.

After that you can probably do gradle run and test it manually.

I'd likely remove all the from IndexResponse#isCreated and UpdateResponse#isCreated and just use that new getOperation method. You could also remove DeleteResponse#isFound but I'm not sure its intuitive. Removing these methods would force you to update the tests to use your changes.

After that I'd hunt down the things that can be removed from the subclass overrides of DocWriteResponse#toXContent and then update the rest tests.

Full disclosure: I probably wouldn't do it in this order but I think this is the right order to do it in. I started trying to implement this myself and bumbled around for 20 minutes before I figured out the order I'd go about it. And I'm used to this code.

No further feedback. Closing

So is this feature not forthcoming? Or did I miss something in reading through this thread?

So is this feature not forthcoming? Or did I miss something in reading through this thread?

It is not. It was closed.

If someone were to finish the development on this, would it be reconsidered?

Sure. We never rejected the feature, just asked for changes to the
implementation.
On Jun 28, 2016 6:33 PM, "Tim Heckel" notifications@github.com wrote:

If someone were to finish the development on this, would it be
reconsidered?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#9736 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AANLokGRHJZWk1rvaEtRMUm5rWmyEGPUks5qQaFDgaJpZM4DhyFj
.