mneethiraj commented on code in PR #667:
URL: https://github.com/apache/ranger/pull/667#discussion_r2350106978
##########
agents-common/src/main/java/org/apache/ranger/plugin/util/PolicyRefresher.java:
##########
@@ -128,23 +128,41 @@ public void setLastActivationTimeInMillis(long
lastActivationTimeInMillis) {
}
public void startRefresher() {
- loadRoles();
- loadPolicy();
-
- super.start();
+ try {
+ loadRoles();
+ } catch (Exception e) {
+ LOG.error("Initial roles load failed for serviceName={}: {}, will
retry in {}ms via scheduled refresh", serviceName, e, pollingIntervalMs);
+ }
+ try {
+ loadPolicy();
+ } catch (Exception e) {
+ LOG.error("Initial policy load failed for serviceName={}: {}, will
retry in {}ms via scheduled refresh", serviceName, e, pollingIntervalMs);
+ }
+ initRefresher();
+ }
+ private void initRefresher() {
+ LOG.debug("==> PolicyRefresher(serviceName={}).initRefresher()",
serviceName);
+ try {
+ super.start();
+ } catch (IllegalStateException e) {
+ LOG.error("Failed to start PolicyRefresher thread for
serviceName={}", serviceName, e);
+ throw e;
+ }
policyDownloadTimer = new Timer("policyDownloadTimer", true);
-
try {
policyDownloadTimer.schedule(new
DownloaderTask(policyDownloadQueue), pollingIntervalMs, pollingIntervalMs);
-
LOG.debug("Scheduled policyDownloadRefresher to download policies
every {} milliseconds", pollingIntervalMs);
- } catch (IllegalStateException exception) {
- LOG.error("Error scheduling policyDownloadTimer:", exception);
+ } catch (IllegalArgumentException | IllegalStateException |
NullPointerException e) {
+ LOG.error("Error scheduling policyDownloadTimer:", e);
LOG.error("*** Policies will NOT be downloaded every {}
milliseconds ***", pollingIntervalMs);
-
- policyDownloadTimer = null;
+ if (policyDownloadTimer != null) {
Review Comment:
Will `policyDownloadTimer` be null here, given the initialization in line
152 above?
##########
agents-common/src/main/java/org/apache/ranger/plugin/util/PolicyRefresher.java:
##########
@@ -128,23 +128,41 @@ public void setLastActivationTimeInMillis(long
lastActivationTimeInMillis) {
}
public void startRefresher() {
- loadRoles();
- loadPolicy();
-
- super.start();
+ try {
Review Comment:
`loadRoles()` and `loadPolicy()` are called from `run()` method as well.
Consider moving `try/catch` blocks inside `loadRoles()` and `loadPolicy()`
implementations.
--
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: dev-unsubscr...@ranger.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
