lhotari commented on code in PR #21764:
URL: https://github.com/apache/pulsar/pull/21764#discussion_r1432325288
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/extensions/store/TableViewLoadDataStoreImpl.java:
##########
@@ -39,7 +39,7 @@ public class TableViewLoadDataStoreImpl<T> implements
LoadDataStore<T> {
private TableView<T> tableView;
- private final Producer<T> producer;
+ private Producer<T> producer;
Review Comment:
Nnow that this isn't a final field anymore, is there a need to make it
volatile for thread safety?
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/extensions/ExtensibleLoadManagerImpl.java:
##########
@@ -789,74 +789,70 @@ public static boolean isInternalTopic(String topic) {
@VisibleForTesting
void playLeader() {
- if (role != Leader) {
- log.info("This broker:{} is changing the role from {} to {}",
- pulsar.getLookupServiceAddress(), role, Leader);
- int retry = 0;
- while (true) {
+ log.info("This broker:{} is setting the role from {} to {}",
+ pulsar.getLookupServiceAddress(), role, Leader);
+ int retry = 0;
+ while (true) {
Review Comment:
Just wondering about a possible shutdown scenario. For those cases, it would
be better to terminate loops based on the thread's interrupt status instead of
having `while(true) {` loops.
```
while (!Thread.currentThread.isInterrupted()) {
```
In addition to this, I'd recommend preserving Thread interrupt status
whenever InterruptedException is caught.
```
try {
Thread.sleep(x);
} catch (InterruptedException e) {
// preserve thread's interrupt status
Thread.currentThread().interrupt();
}
```
(one of the references https://stackoverflow.com/a/52623479)
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/extensions/store/TableViewLoadDataStoreImpl.java:
##########
@@ -39,7 +39,7 @@ public class TableViewLoadDataStoreImpl<T> implements
LoadDataStore<T> {
private TableView<T> tableView;
- private final Producer<T> producer;
+ private Producer<T> producer;
Review Comment:
Now that this isn't a final field anymore, is there a need to make it
volatile for thread safety?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
