Skip to content

HDDS-6701. [Multi-Tenant] Add proper locking between Ranger background sync service and tenant requests; bug fixes #3450

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.

Sign up for GitHub

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

Jump to bottom
Merged
smengcl merged 42 commits into from
May 27, 2022
Merged

HDDS-6701. [Multi-Tenant] Add proper locking between Ranger background sync service and tenant requests; bug fixes #3450

smengcl merged 42 commits into from
May 27, 2022

Conversation

@smengcl
Copy link
Contributor

@smengcl smengcl commented May 23, 2022 •
edited
Loading

What changes were proposed in this pull request?

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-6701

How was this patch tested?

  • Existing tests should pass.

smengcl added 2 commits May 23, 2022 11:46
@smengcl
HDDS-6701. [Multi-Tenant] Add proper locking between Ranger Backgroun…
a7ce295 
…d Sync Service and tenant requests

Change-Id: I6971eafe3936cc7cafb9c5b3592e794ed1163f88
@smengcl
Use a single ReentrantReadWriteLock to protect reads/writes to Rang…
9e634bd 
…er (from the same OM);

Implement Ranger request for delete tenant in preExecute.

Change-Id: I8640f86308cec9e5ea35cff61e755179e4aa759e
@smengcl smengcl requested review from errose28 and prashantpogde May 23, 2022 18:56
smengcl added 26 commits May 23, 2022 21:29
@smengcl
Break up removeTenantFromDBCache from removeTenantFromAuthorizer
49fed64 
Change-Id: I36c889285286c7699ea514bc8961d67d69d35182
@smengcl
HDDS-6600: Limit access ID length to 100
927ddbe 
Change-Id: Ieedf063cde997f308bc46da447f692e3e51267f3
@smengcl
Mark OMMultiTenantManager interface stability as Unstable
d73551c 
Change-Id: If4a4510643d37f10ecc3f14eb8f66353403e0d05
@smengcl
HDDS-6691: allow tenant admins to perform S3 Secret ops on their tena…
5105e62 
…nts' access IDs.

Change-Id: I8189ec59895fa36a58672a3d0bb5e5cf4e40f76b
@smengcl
Merge remote-tracking branch 'asf/HDDS-4944' into HDDS-6701
4edddfc 
Change-Id: I4876075028a37126bc91c4e0cbefb139ea94d79e
@smengcl
Fix DeleteTenant Ranger request delete policies and roles loop
4cb7419 
Change-Id: If5b7dfc145e6d7b1fd69edae6563cf977e0b9288
@smengcl
HDDS-6692: Adjust exception message when accessId is not assigned und…
09d0468 
…er a tenant

Change-Id: I428d83bb3d75320506ac4fcb3f204be18b13a5ef
@smengcl
Unlock authorizer in finally just in case Ranger request throws exc…
20ab352 
…eption

Change-Id: I8396f7d643238688345766cf46034dc8311caf2c
@smengcl
Checkstyle
441d55e 
Change-Id: Ie070f33d243d2b7924a2a4da14289c4916b0acbc
@smengcl
Implement assignTenantAdminInDBCache
62a8250 
Change-Id: I857cea7018d83403c723ad218d491b72222b3361
@smengcl
Implement the InDBCache variant for others tenant requests as well;…
3278569 
… more locks.

Change-Id: I0119aae8d0dbce3e3f611cf23fbe5fcabbe1143a
@smengcl
HDDS-6670: default AssignAdmin to assigning non-delegated admin
23d6288 
Change-Id: I060485acc630c11e510a98a42e491fd6c6ef4d96
@smengcl
Use StampedLock as preExecute and validateAndUpdateCache for th…
7994240 
…e same request (on the same OM) can be run in different threads.

Change-Id: I3ed54438472f574848e6f17bad5f0d3fa56624b8
@smengcl
Catch IllegalMonitorStateException when releasing write lock.
e508b2d 
Change-Id: I9191c155b3bde919208385c3c5271c78b56f7597
@smengcl
Fix UT after the assign admin delegated flag default change
095015b 
Change-Id: I384c256103cf505e2d7d096a9374d7ea3ec1e166
@smengcl
Use withWriteLock and withReadLock in BG Sync.
37446b7 
Change-Id: Ibab088d3b52b4b6067b1ec8739d2af5b7feaabd0
@smengcl
Allow Ranger connection failure during init when getting Ranger cm_oz…
0b2da6e 
…one service version.

Change-Id: I0f896d85519e8853cc7d099ebb4ab5eff71e6f01
@smengcl
Use StampedLock directly rather than StampedLock#asReadWriteLock …
b7133bf 
…to avoid catching `IllegalMonitorStateException`.
@smengcl
Refactor: Move lock methods into new class AuthorizerLock
0d36586 
Change-Id: I3a82c525d80f13b041d2007d9ac51686d66eb7ed
@smengcl
unlockWriteInOMRequest would no longer throw.
bc16e5a 
Change-Id: I551857725cb353cb99d62dbfee5472fcc74359f5
@smengcl
Remove activateTenant/deactivateTenant; clean up
21dead6 
Change-Id: Ia1a552d0a9fed41ec84ad6729a5dd0d5f17cdbcf
@smengcl
Fix UT testGetSecretWithTenant
b01fee8 
Change-Id: I75079536fafd1d4488dabf94ac068b983e894614
@smengcl
Refactor: new classes AuthorizerOp and CacheOp that implements in…
9482f2d 
…terface `TenantOp` to improve maintainability

Change-Id: I8846e48f1aaa2e13a79290e5b78660ea7b301aba
@smengcl
Use String instead of most BasicUserPrincipal; fix UT; re-impleme…
759299d 
…nt `checkAcquiredAuthorizerWriteLock`; ignore `SocketTimeoutException` as well

Change-Id: Iae553c456a7396c5f7f1a64841ac8611ca215720
@smengcl
Clean up
7c670b1 
Change-Id: Icb86a9f3bcf33e229370c433e2a81cf345b3cb21
@smengcl
Fix UT testRevokeUserAccessId
a4dc186 
Change-Id: I71d841ac2618d68f7c0e6bc08defcb77ea053d79
@smengcl
Improve authorizer error handling (e.g. connection issue); Fix accept…
0ed0c72 
…ance test: mock server rule somehow doesn't take wildcard as expected for role request path

Change-Id: I2d58b06d9e06e35ac6b95791f2f233da3d1fda2b
@smengcl smengcl marked this pull request as ready for review May 25, 2022 16:36
@smengcl smengcl changed the title HDDS-6701. [Multi-Tenant] Add proper locking between Ranger Background Sync Service and tenant requests HDDS-6701. [Multi-Tenant] Add proper locking between Ranger background sync service and tenant requests; bug fixes May 25, 2022
@smengcl
Rename proto field TenantRequestLockStamp -> `TenantAuthorizerLockS…
71f8924 
…tamp`.

Change-Id: Ic1a6049d7ddca1f60a32711cf8361e6f51f496e7
prashantpogde
prashantpogde approved these changes May 25, 2022
View reviewed changes
Copy link
Contributor

@prashantpogde prashantpogde left a comment

Choose a reason for hiding this comment

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

LGTM

smengcl added 5 commits May 25, 2022 11:47
@smengcl
Add more comments; javadocs; tweak docs; clean up
7850fbe 
Change-Id: I5b0a155dd8b53690ac11263491ed2e96c85aef16
@smengcl
HDDS-6604: Add UT testCreateTenantOnExistingVolumeShouldFail.
880bacf 
Change-Id: I25a40ede08bc5e2aa1c6eb6ccef7586c55a2d886
@smengcl
HDDS-6600: Add test case of accessId length exceeding limit
2b9b9b4 
Change-Id: I571ad8b639fdd7d9b5d3f702320c3e5ca22d6bd8
@smengcl
Fix BG sync test as we added AuthorizerLock.
c076f5f 
Change-Id: If5d512882df72f5f4cff38af6879f6db3d2679cb
@smengcl
Fix upgrade acceptance check callback. Ref: https://github.com/apache…
00a5007 
…/ozone/pull/3018/files#r882941006

Change-Id: Ia64591aae5f2674d29d6e083f12f5604a1fb87ec
@smengcl smengcl mentioned this pull request May 26, 2022
Merged
5 tasks
smengcl added 5 commits May 26, 2022 14:34
@smengcl
The lock stamp will be stored solely inside the lock instance; remove…
462cdbc 
… proto field `TenantAuthorizerLockStamp`; minor tweaks

Change-Id: I309578ca1618e2fe62563ee7aca72280b626cdbe
@smengcl
Add UT TestAuthorizerLockImpl; tweaks; test fixes
c231107 
Change-Id: Ib3f5f384e4d993b6bd462052b4e1fe5e8bd23ec3
@smengcl
Improve BG Sync error handling
a873bb2 
Change-Id: Id1e91387c44979cc4fb06a5e2eb7bcecc6d2c182
@smengcl
Suppress false positive findbugs warning of intended IMSE in UT.
e4d9a03 
Change-Id: Iacf8a887c87e42bea59d00d9150f344caac9627d
@smengcl
Move AuthorizerLock and Impl under .multitenant package
61ec09d 
Change-Id: I41240f3322b0ae0ac328203f9d7ec64a06af28d1
errose28
errose28 reviewed May 27, 2022
View reviewed changes
Copy link
Contributor

@errose28 errose28 left a comment

Choose a reason for hiding this comment

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

Thanks for working on this @smengcl. Just some minor comments.

smengcl added 2 commits May 26, 2022 17:54
@smengcl
Address Ethan's comments
1737862 
Change-Id: I83633f7dba43edfa264ab020743c6ed13ca01bba
@smengcl
Make tryWriteLockInOMRequest return type void
b1d8963 
Change-Id: I62168184c6ab9cc63b517dddb775d4df5bcc6d8b
@smengcl
Copy link
Contributor Author

smengcl commented May 27, 2022

Thanks @errose28 for the review. I have addressed the comments. Please take another look, thx!

errose28
errose28 approved these changes May 27, 2022
View reviewed changes
Copy link
Contributor

@errose28 errose28 left a comment

Choose a reason for hiding this comment

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

Thanks for the updates, LGTM.

@smengcl
Copy link
Contributor Author

smengcl commented May 27, 2022

Thanks @prashantpogde @errose28 for the review.

@smengcl smengcl merged commit dcd1739 into apache:HDDS-4944 May 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@prashantpogde prashantpogde prashantpogde approved these changes

@errose28 errose28 errose28 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@smengcl @prashantpogde @errose28