Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Disable Annotation RPC #873

Closed
wants to merge 3 commits into from
Closed

Conversation

johann8384
Copy link
Member

Maybe this will work for you to disable support for annotation requests? @thatsafunnyname
Fix #823

@johann8384
Copy link
Member Author

@thatsafunnyname This includes your patch from #823 as well as disables the Annotation RPC.

@thatsafunnyname
Copy link
Contributor

thatsafunnyname commented Oct 7, 2016

Thank you for this. I just added #823 (comment) , in it I report seeing CPUs busy and threads using annotations code in processRow in SaltScanner.java, I am not sure if your changes would avoid this code path, or if an additional guard would be needed around this code.

@thatsafunnyname
Copy link
Contributor

I was wondering if this will get into a 2.3.0 or 2.3.1 release? Thanks.

@johann8384
Copy link
Member Author

johann8384 commented Oct 19, 2016

Did it solve that issue for you? Of course it is just a workaround, there
is an actual issue we need to fix in the code we are bypassing.

@thatsafunnyname
Copy link
Contributor

I have not tested it. Will it avoid these lines in processRow in SaltScanner.java from being run?

493      List<Annotation> notes = annotations.get(key);
494      if (notes == null) {
495        notes = new ArrayList<Annotation>();
496        annotations.put(key, notes);
497      }

It was removing this code that fixed the issue. Thanks.

@johann8384 johann8384 added this to the v2.3.0 milestone Dec 2, 2016
@johann8384
Copy link
Member Author

I need to spend a little time on this and come back to it.

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants