Re: [PR] HADOOP-18851. DT performance improvement latest review incorporation [hadoop]

2024-05-14 Thread via GitHub
hadoop-yetus commented on PR #6803: URL: https://github.com/apache/hadoop/pull/6803#issuecomment-2111003939 :broken_heart: **-1 overall** | Vote | Subsystem | Runtime | Logfile | Comment | |::|--:|:|::|:---:| _ Prechecks

Re: [PR] HADOOP-18851. DT performance improvement latest review incorporation [hadoop]

2024-05-14 Thread via GitHub
ChenSammi commented on PR #6803: URL: https://github.com/apache/hadoop/pull/6803#issuecomment-2110134515 @Hexiaoqiao , thanks for the info about mvnsite. Current, hadoop-yetus is passed. This "Apache Yetus", I never see it's all green. Should this github CI be all green before the PR can

Re: [PR] HADOOP-18851. DT performance improvement latest review incorporation [hadoop]

2024-05-14 Thread via GitHub
hadoop-yetus commented on PR #6803: URL: https://github.com/apache/hadoop/pull/6803#issuecomment-2109897935 :broken_heart: **-1 overall** | Vote | Subsystem | Runtime | Logfile | Comment | |::|--:|:|::|:---:| | +0 :ok: |

Re: [PR] HADOOP-18851. DT performance improvement latest review incorporation [hadoop]

2024-05-13 Thread via GitHub
Hexiaoqiao commented on PR #6803: URL: https://github.com/apache/hadoop/pull/6803#issuecomment-2109216164 > the mvnsite failure is, looks like not relevant, but other merged MR seems doesn't have this failure. Hi @ChenSammi, @vikaskr22 `mvnsite` failure is not related to this PR, it

Re: [PR] HADOOP-18851. DT performance improvement latest review incorporation [hadoop]

2024-05-13 Thread via GitHub
hadoop-yetus commented on PR #6803: URL: https://github.com/apache/hadoop/pull/6803#issuecomment-2108044944 :broken_heart: **-1 overall** | Vote | Subsystem | Runtime | Logfile | Comment | |::|--:|:|::|:---:| _ Prechecks

Re: [PR] HADOOP-18851. DT performance improvement latest review incorporation [hadoop]

2024-05-13 Thread via GitHub
hadoop-yetus commented on PR #6803: URL: https://github.com/apache/hadoop/pull/6803#issuecomment-2107268466 :broken_heart: **-1 overall** | Vote | Subsystem | Runtime | Logfile | Comment | |::|--:|:|::|:---:| | +0 :ok: |

Re: [PR] HADOOP-18851. DT performance improvement latest review incorporation [hadoop]

2024-05-13 Thread via GitHub
ChenSammi commented on PR #6803: URL: https://github.com/apache/hadoop/pull/6803#issuecomment-2106842836 This mvnsite failure is, looks like not relevant, but other merged MR seems doesn't have this failure. ``` [INFO] --- exec-maven-plugin:1.3.1:exec (shelldocs) @ hadoop-common ---

Re: [PR] HADOOP-18851. DT performance improvement latest review incorporation [hadoop]

2024-05-13 Thread via GitHub
Hexiaoqiao commented on PR #6803: URL: https://github.com/apache/hadoop/pull/6803#issuecomment-2106824252 @ChenSammi Got it. Make sense to me. > Yes, removing the synchronized directly have potential risks to sub classes of AbstractDelegationTokenSecretManager. Using LOCK is a safe way

Re: [PR] HADOOP-18851. DT performance improvement latest review incorporation [hadoop]

2024-05-13 Thread via GitHub
vikaskr22 commented on PR #6803: URL: https://github.com/apache/hadoop/pull/6803#issuecomment-2106765757 @Hexiaoqiao , @ChenSammi , Anything else needed from my side? I had fixed all the review comments that's part of my commit. -- This is an automated message from the Apache Git

Re: [PR] HADOOP-18851. DT performance improvement latest review incorporation [hadoop]

2024-05-11 Thread via GitHub
hadoop-yetus commented on PR #6803: URL: https://github.com/apache/hadoop/pull/6803#issuecomment-2105598897 :broken_heart: **-1 overall** | Vote | Subsystem | Runtime | Logfile | Comment | |::|--:|:|::|:---:| _ Prechecks

Re: [PR] HADOOP-18851. DT performance improvement latest review incorporation [hadoop]

2024-05-10 Thread via GitHub
hadoop-yetus commented on PR #6803: URL: https://github.com/apache/hadoop/pull/6803#issuecomment-2104796915 :broken_heart: **-1 overall** | Vote | Subsystem | Runtime | Logfile | Comment | |::|--:|:|::|:---:| | +0 :ok: |

Re: [PR] HADOOP-18851. DT performance improvement latest review incorporation [hadoop]

2024-05-10 Thread via GitHub
hadoop-yetus commented on PR #6803: URL: https://github.com/apache/hadoop/pull/6803#issuecomment-2104125153 :broken_heart: **-1 overall** | Vote | Subsystem | Runtime | Logfile | Comment | |::|--:|:|::|:---:| _ Prechecks

Re: [PR] HADOOP-18851. DT performance improvement latest review incorporation [hadoop]

2024-05-09 Thread via GitHub
ChenSammi commented on PR #6803: URL: https://github.com/apache/hadoop/pull/6803#issuecomment-2103782158 > @ChenSammi , Please add your input as well in case I missed anything. Thanks. Yes, removing the synchronized directly have potential risks to sub classes of

Re: [PR] HADOOP-18851. DT performance improvement latest review incorporation [hadoop]

2024-05-09 Thread via GitHub
vikaskr22 commented on PR #6803: URL: https://github.com/apache/hadoop/pull/6803#issuecomment-2102594497 Hi @Hexiaoqiao , technically I didn't see or observe any issues till now but @ChenSammi has some concerns related to concurrency in other sub classes of

Re: [PR] HADOOP-18851. DT performance improvement latest review incorporation [hadoop]

2024-05-08 Thread via GitHub
Hexiaoqiao commented on PR #6803: URL: https://github.com/apache/hadoop/pull/6803#issuecomment-2101893364 Hi @vikaskr22 @ChenSammi Thanks for your works here. One nit concerns, #6001 try to remove synchronization then revert and add RWLock here, any issues do you meet while remove

Re: [PR] HADOOP-18851. DT performance improvement latest review incorporation [hadoop]

2024-05-08 Thread via GitHub
ChenSammi commented on code in PR #6803: URL: https://github.com/apache/hadoop/pull/6803#discussion_r1594856131 ## hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/delegation/AbstractDelegationTokenSecretManager.java: ## @@ -169,21 +172,29 @@

Re: [PR] HADOOP-18851. DT performance improvement latest review incorporation [hadoop]

2024-05-08 Thread via GitHub
ChenSammi commented on code in PR #6803: URL: https://github.com/apache/hadoop/pull/6803#discussion_r1594856131 ## hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/delegation/AbstractDelegationTokenSecretManager.java: ## @@ -169,21 +172,29 @@

Re: [PR] HADOOP-18851. DT performance improvement latest review incorporation [hadoop]

2024-05-08 Thread via GitHub
ChenSammi commented on code in PR #6803: URL: https://github.com/apache/hadoop/pull/6803#discussion_r1594856131 ## hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/delegation/AbstractDelegationTokenSecretManager.java: ## @@ -169,21 +172,29 @@

Re: [PR] HADOOP-18851. DT performance improvement latest review incorporation [hadoop]

2024-05-08 Thread via GitHub
hadoop-yetus commented on PR #6803: URL: https://github.com/apache/hadoop/pull/6803#issuecomment-2100256835 :broken_heart: **-1 overall** | Vote | Subsystem | Runtime | Logfile | Comment | |::|--:|:|::|:---:| | +0 :ok: |

Re: [PR] HADOOP-18851. DT performance improvement latest review incorporation [hadoop]

2024-05-08 Thread via GitHub
vikaskr22 commented on code in PR #6803: URL: https://github.com/apache/hadoop/pull/6803#discussion_r1593797928 ## hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/delegation/AbstractDelegationTokenSecretManager.java: ## @@ -441,18 +496,14 @@

Re: [PR] HADOOP-18851. DT performance improvement latest review incorporation [hadoop]

2024-05-08 Thread via GitHub
vikaskr22 commented on code in PR #6803: URL: https://github.com/apache/hadoop/pull/6803#discussion_r1593797668 ## hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/delegation/AbstractDelegationTokenSecretManager.java: ## @@ -169,21 +172,29 @@

Re: [PR] HADOOP-18851. DT performance improvement latest review incorporation [hadoop]

2024-05-07 Thread via GitHub
hadoop-yetus commented on PR #6803: URL: https://github.com/apache/hadoop/pull/6803#issuecomment-2099774430 :broken_heart: **-1 overall** | Vote | Subsystem | Runtime | Logfile | Comment | |::|--:|:|::|:---:| _ Prechecks

Re: [PR] HADOOP-18851. DT performance improvement latest review incorporation [hadoop]

2024-05-07 Thread via GitHub
ChenSammi commented on code in PR #6803: URL: https://github.com/apache/hadoop/pull/6803#discussion_r1593369308 ## hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/delegation/AbstractDelegationTokenSecretManager.java: ## @@ -169,21 +172,29 @@

Re: [PR] HADOOP-18851. DT performance improvement latest review incorporation [hadoop]

2024-05-07 Thread via GitHub
ChenSammi commented on code in PR #6803: URL: https://github.com/apache/hadoop/pull/6803#discussion_r1593368398 ## hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/delegation/AbstractDelegationTokenSecretManager.java: ## @@ -120,12 +121,12 @@

Re: [PR] HADOOP-18851. DT performance improvement latest review incorporation [hadoop]

2024-05-07 Thread via GitHub
ChenSammi commented on code in PR #6803: URL: https://github.com/apache/hadoop/pull/6803#discussion_r1593339527 ## hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/delegation/AbstractDelegationTokenSecretManager.java: ## @@ -441,18 +496,14 @@

Re: [PR] HADOOP-18851. DT performance improvement latest review incorporation [hadoop]

2024-05-07 Thread via GitHub
ChenSammi commented on code in PR #6803: URL: https://github.com/apache/hadoop/pull/6803#discussion_r1593339527 ## hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/delegation/AbstractDelegationTokenSecretManager.java: ## @@ -441,18 +496,14 @@

Re: [PR] HADOOP-18851. DT performance improvement latest review incorporation [hadoop]

2024-05-07 Thread via GitHub
ChenSammi commented on code in PR #6803: URL: https://github.com/apache/hadoop/pull/6803#discussion_r1593339527 ## hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/delegation/AbstractDelegationTokenSecretManager.java: ## @@ -441,18 +496,14 @@

Re: [PR] HADOOP-18851: DT performance improvement latest review incorporation [hadoop]

2024-05-07 Thread via GitHub
hadoop-yetus commented on PR #6803: URL: https://github.com/apache/hadoop/pull/6803#issuecomment-2098357532 :broken_heart: **-1 overall** | Vote | Subsystem | Runtime | Logfile | Comment | |::|--:|:|::|:---:| | +0 :ok: |

Re: [PR] HADOOP-18851: DT performance improvement latest review incorporation [hadoop]

2024-05-07 Thread via GitHub
vikaskr22 commented on PR #6803: URL: https://github.com/apache/hadoop/pull/6803#issuecomment-2098067781 @ChenSammi , Can you review the changes and provide your input ? Thanks. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub

[PR] HADOOP-18851: DT performance improvement latest review incorporation [hadoop]

2024-05-07 Thread via GitHub
vikaskr22 opened a new pull request, #6803: URL: https://github.com/apache/hadoop/pull/6803 ### Description of PR AbstractDelegationTokenSecretManager's method all synchronized and are blocking each other, even multiple readers threads are blocking each other. This PR is an