Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Add AdmissionController interface #3567

Closed
wants to merge 34 commits into from
Closed

Add AdmissionController interface #3567

wants to merge 34 commits into from

Conversation

mitalawachat
Copy link

Signed-off-by: Mital Awachat [email protected]

Description

This pull request serves as a starting point for Admission Control. In this pull request we are adding AdmissionController interface which will be fulfilled by concrete implementation in subsequent pull requests as mentioned in design proposal given below.

Issues Resolved

Feature Proposal: #1144
Design Proposal: #3400

Check List

  • [-] New functionality includes testing.
    • [-] All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@mitalawachat mitalawachat requested review from a team and reta as code owners June 13, 2022 13:02
@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 647bd91e099333ec252bd3ec1010aefddb892749
Log 5924

Reports 5924

Comment on lines 27 to 40

/**
* Increment the tracking-object with provided value.
* Apply the admission control if threshold is breached.
* Mostly applicable while acquiring the quota.
*
* @param count value to incrementation the resource racking-object with.
* @return count/value acquired from the resource tracking object.
*/
long acquire(long count);

/**
* Decrement the tracking-object with provided value and do not apply the admission control.
* Mostly applicable while remitting the quota.
*
* @param count value to decrement the resource tracking-object with.
* @return count/value released to the resource tracking object.
*/
long release(long count);

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are internal working of an Admission controller, so doesn't add value exposing them

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Bukhtawar,
Thanks for reviewing pull request.

I was planning to create AdmissionControlHandler, a netty handler that extends ChannelDuplexHandler.

  • acquire function would be used from channelRead function of AdmissionControlHandler
  • release function would be used from write function of AdmissionControlHandler

Let me know your thoughts about it.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me rephrase it on why I call them internal working
For a particular request, release cannot release more than what it acquired. Recommend having a method addBytesAndMaybeBreak

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Bukhtawar. Instead of separate function for acquire and release, I've added addBytesAndMaybeBreak function which returns Releasable which would be used to release tokens which were allocated to a particular request.

@mitalawachat mitalawachat requested a review from Bukhtawar June 16, 2022 18:08
@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 5ce3030e1ff7deea01f392297ac984258de9aff8
Log 6359

Reports 6359

mitalawachat and others added 24 commits June 27, 2022 23:47
…#3571)

* Bump jettison from 1.4.1 to 1.5.0 in /plugins/discovery-azure-classic

Bumps [jettison](https://github.com/jettison-json/jettison) from 1.4.1 to 1.5.0.
- [Release notes](https://github.com/jettison-json/jettison/releases)
- [Commits](jettison-json/jettison@jettison-1.4.1...jettison-1.5.0)

---
updated-dependencies:
- dependency-name: org.codehaus.jettison:jettison
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>

* Updating SHAs

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: dependabot[bot] <dependabot[bot]@users.noreply.github.com>
…v20220608-1.32.1 in /plugins/repository-gcs (#3573)

* Bump google-api-services-storage in /plugins/repository-gcs

Bumps google-api-services-storage from v1-rev20200814-1.30.10 to v1-rev20220608-1.32.1.

---
updated-dependencies:
- dependency-name: com.google.apis:google-api-services-storage
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>

* Updating SHAs

Signed-off-by: dependabot[bot] <[email protected]>

* Upgrade Google HTTP Client to 1.42.0

Signed-off-by: Xue Zhou <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: dependabot[bot] <dependabot[bot]@users.noreply.github.com>
Co-authored-by: Xue Zhou <[email protected]>
* Add flat_skew setting to node overload decider

Signed-off-by: Rishab Nahata <[email protected]>
* Bump xmlbeans from 5.0.3 to 5.1.0 in /plugins/ingest-attachment

Bumps xmlbeans from 5.0.3 to 5.1.0.

---
updated-dependencies:
- dependency-name: org.apache.xmlbeans:xmlbeans
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>

* Updating SHAs

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: dependabot[bot] <dependabot[bot]@users.noreply.github.com>
…gce (#3570)

* Bump google-oauth-client from 1.34.0 to 1.34.1 in /plugins/discovery-gce

Bumps [google-oauth-client](https://github.com/googleapis/google-oauth-java-client) from 1.34.0 to 1.34.1.
- [Release notes](https://github.com/googleapis/google-oauth-java-client/releases)
- [Changelog](https://github.com/googleapis/google-oauth-java-client/blob/main/CHANGELOG.md)
- [Commits](googleapis/google-oauth-java-client@v1.34.0...v1.34.1)

---
updated-dependencies:
- dependency-name: com.google.oauth-client:google-oauth-client
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

* Updating SHAs

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: dependabot[bot] <dependabot[bot]@users.noreply.github.com>
…AllocationDecider (#3428)

* Fix for bug showing incorrect awareness attributes count in AwarenessAllocationDecider

Signed-off-by: Anshu Agarwal <[email protected]>
Signed-off-by: GitHub <[email protected]>

Co-authored-by: opensearch-ci-bot <[email protected]>
* Support unknown node role

Currently OpenSearch only supports several built-in nodes like data node
role. If specify unknown node role, OpenSearch node will fail to start.
This limit how to extend OpenSearch to support some extension function.
For example, user may prefer to run ML tasks on some dedicated node
which doesn't serve as any built-in node roles. So the ML tasks won't
impact OpenSearch core function. This PR removed the limitation and user
can specify any node role and OpenSearch will start node correctly with
that unknown role. This opens the door for plugin developer to run
specific tasks on dedicated nodes.

Issue: #2877

Signed-off-by: Yaliang Wu <[email protected]>

* fix cat nodes rest API spec

Signed-off-by: Yaliang Wu <[email protected]>

* fix mixed cluster IT failure

Signed-off-by: Yaliang Wu <[email protected]>

* add DynamicRole

Signed-off-by: Yaliang Wu <[email protected]>

* change generator method name

Signed-off-by: Yaliang Wu <[email protected]>

* fix failed docker test

Signed-off-by: Yaliang Wu <[email protected]>

* transform role name to lower case to avoid confusion

Signed-off-by: Yaliang Wu <[email protected]>

* transform the node role abbreviation to lower case

Signed-off-by: Yaliang Wu <[email protected]>

* fix checkstyle

Signed-off-by: Yaliang Wu <[email protected]>

* add test for case-insensitive role name change

Signed-off-by: Yaliang Wu <[email protected]>
…stermanager' (#3556)

* Rename package org.opensearch.action.support.master to org.opensearch.action.support.clustermanager

Signed-off-by: Tianli Feng <[email protected]>

* Rename classes with master term in the package org.opensearch.action.support.master

Signed-off-by: Tianli Feng <[email protected]>

* Deprecate classes in org.opensearch.action.support.master

Signed-off-by: Tianli Feng <[email protected]>

* Remove pakcage o.o.action.support.master

Signed-off-by: Tianli Feng <[email protected]>

* Move package-info back

Signed-off-by: Tianli Feng <[email protected]>

* Move package-info to new folder

Signed-off-by: Tianli Feng <[email protected]>

* Correct the package-info

Signed-off-by: Tianli Feng <[email protected]>
Co-authored-by: opensearch-ci-bot <[email protected]>
* Fix false positive query timeouts due to using cached time

Signed-off-by: Ahmad AbuKhalil <[email protected]>

* delegate nanoTime call to SearchContext

Signed-off-by: Ahmad AbuKhalil <[email protected]>

* add override to SearchContext getRelativeTimeInMillis to force non cached time

Signed-off-by: Ahmad AbuKhalil <[email protected]>
…rm file copy. (#3525)

* Add components for segment replication to perform file copy.

This change adds the required components to SegmentReplicationSourceService to initiate copy and react to lifecycle events.
Along with new components it refactors common file copy code from RecoverySourceHandler into reusable pieces.

Signed-off-by: Marc Handalian <[email protected]>
…rg.opensearch.action.support.master' (#3617)

Signed-off-by: Tianli Feng <[email protected]>
* implement segment replication target

Signed-off-by: Poojita Raj <[email protected]>

* test added

Signed-off-by: Poojita Raj <[email protected]>

* changes to tests + finalizeReplication

Signed-off-by: Poojita Raj <[email protected]>

* fix style check

Signed-off-by: Poojita Raj <[email protected]>

* addressing comments + fix gradle check

Signed-off-by: Poojita Raj <[email protected]>

* added test + addressed review comments

Signed-off-by: Poojita Raj <[email protected]>
…ply (#3626)

* [BUG] opensearch crashes on closed client connection before search reply

Signed-off-by: Andriy Redko <[email protected]>

* Addressing code review comments

Signed-off-by: Andriy Redko <[email protected]>
…h.action.support.clustermanager' (#3644)

Signed-off-by: Tianli Feng <[email protected]>
…3638)

* Introduce decoupled translog manager interfaces

Signed-off-by: Bukhtawar Khan <[email protected]>
Rishikesh1159 and others added 10 commits June 27, 2022 23:47
…ment Replication is turned on (#3540)

* Adding onNewCheckpoint and it's test to start replication. SCheck for latestcheckpoint and replaying logic is removed from this commit and will be added in a different PR

Signed-off-by: Rishikesh1159 <[email protected]>

* Changing binding/inject logic and addressing comments from PR

Signed-off-by: Rishikesh1159 <[email protected]>

* Applying spotless check

Signed-off-by: Rishikesh1159 <[email protected]>

* Moving shouldProcessCheckpoint() to IndexShard, and removing some trace logs

Signed-off-by: Rishikesh1159 <[email protected]>

* applying spotlessApply

Signed-off-by: Rishikesh1159 <[email protected]>

* Adding more info to log statement in targetservice class

Signed-off-by: Rishikesh1159 <[email protected]>

* applying spotlessApply

Signed-off-by: Rishikesh1159 <[email protected]>

* Addressing comments on PR

Signed-off-by: Rishikesh1159 <[email protected]>

* Adding teardown() in SegmentReplicationTargetServiceTests.

Signed-off-by: Rishikesh1159 <[email protected]>

* fixing testShouldProcessCheckpoint() in SegmentReplicationTargetServiceTests

Signed-off-by: Rishikesh1159 <[email protected]>

* Removing CheckpointPublisherProvider in IndicesModule

Signed-off-by: Rishikesh1159 <[email protected]>

* spotless check apply

Signed-off-by: Rishikesh1159 <[email protected]>
#3662)

* Remove class org.opensearch.action.support.master.AcknowledgedResponse

Signed-off-by: Tianli Feng <[email protected]>

* Remove class org.opensearch.action.support.master.AcknowledgedRequest RequestBuilder ShardsAcknowledgedResponse

Signed-off-by: Tianli Feng <[email protected]>
…pensearch.action.support.master (#3669)

Signed-off-by: Tianli Feng <[email protected]>
…tags (url, scm) (#3656)

* [BUG] Custom POM configuration for ZIP publication produces duplicit tags (url, scm)

Signed-off-by: Andriy Redko <[email protected]>

* Added test case for pluginZip with POM

Signed-off-by: Andriy Redko <[email protected]>

* Support both Gradle 6.8.x and Gradle 7.4.x

Signed-off-by: Andriy Redko <[email protected]>
…reness (#3646)

* Fixing flaky test org.opensearch.cluster.allocation.AwarenessAllocationIT.testThreeZoneOneReplicaWithForceZoneValueAndLoadAwareness by adding dedicated cluster manager node

Signed-off-by: Rishab Nahata <[email protected]>
@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 877f06adc75929ba9762f3319232e4c06211c805
Log 6374

Reports 6374

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure bf4462a
Log 6375

Reports 6375

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.