markap14 commented on code in PR #7266:
URL: https://github.com/apache/nifi/pull/7266#discussion_r1202717982
##########
nifi-nar-bundles/nifi-enrich-bundle/nifi-enrich-processors/src/main/java/org/apache/nifi/processors/AbstractEnrichIP.java:
##########
@@ -77,6 +82,11 @@ public abstract class AbstractEnrichIP extends
AbstractProcessor {
private Set<Relationship> relationships;
private List<PropertyDescriptor> propertyDescriptors;
final AtomicReference<DatabaseReader> databaseReaderRef = new
AtomicReference<>(null);
+ protected volatile SynchronousFileWatcher watcher;
+
+ protected final Lock dbWriteLock = new ReentrantLock();
+
+ protected volatile File dbFile;
Review Comment:
We should avoid having `protected` member variables - these should be
`private` with getters
##########
nifi-nar-bundles/nifi-enrich-bundle/nifi-enrich-processors/src/main/java/org/apache/nifi/processors/GeoEnrichIP.java:
##########
@@ -97,10 +112,36 @@ public void onTrigger(final ProcessContext context, final
ProcessSession session
return;
}
- final StopWatch stopWatch = new StopWatch(true);
+ StopWatch stopWatch = new StopWatch(true);
try {
response = dbReader.city(inetAddress);
stopWatch.stop();
+ } catch (InvalidDatabaseException idbe) {
+ stopWatch.stop();
+ if (dbWriteLock.tryLock()) {
+ try {
+ getLogger().debug("Attempting to reload database after
InvalidDatabaseException");
+ try {
+ loadDatabaseFile();
+ } catch (IOException ioe) {
+ throw new ProcessException("Error reloading database
due to: " + ioe.getMessage(), ioe);
+ }
+
+ getLogger().debug("Attempting to retry lookup after
InvalidDatabaseException");
+ try {
+ dbReader = databaseReaderRef.get();
+ stopWatch = new StopWatch(true);
+ response = dbReader.city(inetAddress);
+ stopWatch.stop();
+ } catch (final Exception e) {
+ throw new ProcessException("Error performing look up:
" + e.getMessage(), e);
+ }
+ } finally {
+ dbWriteLock.unlock();
+ }
+ } else {
+ throw new ProcessException("Failed to lookup the key " +
inetAddress + " due to " + idbe.getMessage(), idbe);
+ }
Review Comment:
This is a lot of complexity for Exception handling, and its quite repetitive
of what's been done above. I think I'd be tempted to instead re-queue the
FlowFIle (either by calling `session.transfer(flowFile);` or
`session.rollback();`) and just setting a `needsReload` member variable or
something, so that on the next invocation the database gets reloaded. Any
thoughts on that approach?
##########
nifi-nar-bundles/nifi-enrich-bundle/nifi-enrich-processors/src/main/java/org/apache/nifi/processors/GeoEnrichIPRecord.java:
##########
@@ -227,12 +241,38 @@ public void onTrigger(ProcessContext context,
ProcessSession session) throws Pro
int foundCount = 0;
int notFoundCount = 0;
while ((record = reader.nextRecord()) != null) {
- CityResponse response = geocode(ipPath, record, dbReader);
+ CityResponse response;
+ try {
+ response = geocode(ipPath, record, dbReader);
+ } catch (InvalidDatabaseException idbe) {
+ if (dbWriteLock.tryLock()) {
+ try {
+ getLogger().debug("Attempting to reload database
after InvalidDatabaseException");
+ try {
+ loadDatabaseFile();
+ } catch (IOException ioe) {
+ throw new ProcessException("Error reloading
database due to: " + ioe.getMessage(), ioe);
+ }
+
+ getLogger().debug("Attempting to retry lookup
after InvalidDatabaseException");
+ try {
+ dbReader = databaseReaderRef.get();
+ response = geocode(ipPath, record, dbReader);
+ } catch (final Exception e) {
+ throw new ProcessException("Error performing
look up: " + e.getMessage(), e);
+ }
+ } finally {
+ dbWriteLock.unlock();
+ }
+ } else {
+ throw new ProcessException("Failed to lookup the key
from " + ipPath.getPath() + " due to " + idbe.getMessage(), idbe);
+ }
Review Comment:
Same comment as above, probably makes sense to rollback and try again
##########
nifi-nar-bundles/nifi-enrich-bundle/nifi-enrich-processors/src/main/java/org/apache/nifi/processors/GeoEnrichIP.java:
##########
@@ -72,14 +73,28 @@ public void onTrigger(final ProcessContext context, final
ProcessSession session
return;
}
- final DatabaseReader dbReader = databaseReaderRef.get();
+ try {
+ if (watcher != null && watcher.checkAndReset()) {
Review Comment:
`watcher` will never be null here
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
