Skip to content

Latest commit

 

History

History
292 lines (213 loc) · 11.9 KB

adr-011-lazy-initialization-event-handler.md

File metadata and controls

292 lines (213 loc) · 11.9 KB

ADR-011: Lazy Initialization for SearchIndexEventHandler

Status: Accepted Date: 2024-12-24 (Initial), 2024-12-29 (Corrected), 2024-12-29 (Optimized) Issue: CLD-1677 (startup NullPointerException + chicken-and-egg problem) Deciders: CloudEmpiere Development Team

Context

Background

The SearchIndexEventHandler OSGi Declarative Services component was failing during Eclipse/iDempiere startup with a NullPointerException:

SEVERE: No Database Connection
java.lang.NullPointerException
    at org.compiere.db.PreparedStatementProxy.<init>(PreparedStatementProxy.java:41)
    at com.cloudempiere.searchindex.util.SearchIndexUtils.getSearchIndexConfigs(SearchIndexUtils.java:170)
    at com.cloudempiere.searchindex.event.SearchIndexEventHandler.initialize(SearchIndexEventHandler.java:69)
    at org.adempiere.base.event.AbstractEventHandler.bindEventManager(AbstractEventHandler.java:58)

The root cause:

  1. OSGi SCR activates SearchIndexEventHandler when IEventManager service becomes available
  2. The bindEventManager() method calls initialize() which immediately queries the database
  3. The database connection pool may not be initialized yet at this point in startup
  4. This causes a NullPointerException when trying to execute SQL

Requirements

  • Event handler must start without errors during OSGi startup
  • Event handler must function correctly once database is available
  • Solution must work across all environments: Eclipse IDE, server, Docker, cloud
  • Must follow iDempiere established patterns

Decision Drivers

  • Reliability: Must not fail during startup regardless of timing
  • Compatibility: Must work in all deployment scenarios
  • Maintainability: Should follow existing iDempiere patterns
  • Simplicity: Avoid complex configuration or external dependencies

Considered Options

Option 1: Lazy Initialization with DB.isConnected() Check

Description: Defer database queries until DB.isConnected() returns true. Register static table events in initialize(), and dynamically register indexed tables on first event when DB is ready.

Pros:

  • Follows established iDempiere pattern (used in Msg.java, Language.java, EMail.java, Env.java)
  • Self-healing: automatically recovers when DB becomes available
  • Works in all environments without configuration changes
  • Thread-safe with double-checked locking

Cons:

  • Slight delay on first event after DB is ready
  • Adds complexity to event handler code

Cost/Effort: Low

Option 2: OSGi Service Dependency on AdempiereDatabase

Description: Add @Reference to AdempiereDatabase service to ensure component only activates when database service is available.

Pros:

  • Uses OSGi dependency injection
  • Explicit service dependency

Cons:

  • Database service availability doesn't guarantee connection pool is ready
  • Would not solve the root cause
  • Requires MANIFEST.MF changes

Cost/Effort: Low

Option 3: OSGi Start Level Configuration

Description: Configure bundle start levels to ensure searchindex bundle starts after database bundle.

Pros:

  • Standard OSGi approach for bundle ordering

Cons:

  • Start levels only control bundle start order, not DS component activation
  • DS components activate when service references are satisfied, not when bundle starts
  • Harder to maintain across deployments
  • Environment-specific configuration required

Cost/Effort: Medium

Option 4: Server Start Event Listener

Description: Listen for iDempiere server start event and initialize tables then.

Pros:

  • Clean separation of concerns
  • Guaranteed platform is fully ready

Cons:

  • May not fire in Eclipse IDE development mode
  • Only works in server context
  • Would still need fallback for non-server contexts
  • More complex event wiring

Cost/Effort: Medium

Decision

We will use simple Thread with exponential backoff for lazy initialization. This is simpler and more efficient than ScheduledExecutorService for one-time startup polling.

Initial Implementation Problem (Commit 328a8bd)

The initial lazy initialization approach had a critical flaw:

  1. initialize() only registered static config tables (AD_SearchIndex, AD_SearchIndexTable, AD_SearchIndexColumn)
  2. Dynamic tables (M_Product, C_Order, etc.) were supposed to be registered lazily via ensureTablesRegistered()
  3. ensureTablesRegistered() was called from doHandleEvent()
  4. BUT doHandleEvent() only fires for events on already registered tables
  5. Chicken-and-egg: Dynamic tables were never registered because events for unregistered tables don't fire

Corrected Solution (v1 - ScheduledExecutorService)

Initial fix used ScheduledExecutorService with 100ms polling intervals. This worked but had issues:

  • Required complex shutdown logic
  • Two separate scheduled tasks (polling + timeout)
  • Overhead of executor framework for one-time use

Optimized Solution (v2 - Simple Thread with Exponential Backoff)

Use simple Thread with exponential backoff polling:

  1. In initialize(), try immediate registration if DB.isConnected() is true
  2. If DB not ready, start daemon thread with exponential backoff (10ms → 500ms)
  3. Thread exits naturally when DB connects or 60s timeout reached
  4. No cleanup needed - daemon thread doesn't prevent JVM shutdown

Rationale

  1. Fixes Chicken-and-Egg: Background thread ensures dynamic tables are always registered once DB is ready, regardless of event triggers

  2. Exponential Backoff Performance:

    • 0ms delay if DB already connected when plugin starts
    • 10-20ms average delay if DB connects quickly (exponential backoff starts at 10ms)
    • Max 500ms intervals after backoff progression
    • Much better than fixed 100ms polling

  3. Simpler Code:

    • No ScheduledExecutorService complexity
    • No shutdownScheduler() method needed
    • Thread exits naturally when done
    • Fewer moving parts = fewer bugs

  4. Resource Efficiency:

    • Single daemon thread (auto-cleanup)
    • Exponential backoff reduces CPU overhead
    • No executor framework overhead for one-time operation

Consequences

Positive

  • ✅ Startup no longer fails with NullPointerException
  • ✅ Fixes chicken-and-egg problem - dynamic tables always registered
  • ✅ Zero delay if DB is already ready (most common case)
  • ✅ 10-20ms average delay if DB not ready (exponential backoff)
  • ✅ Works across all deployment environments
  • ✅ Simpler code - no ScheduledExecutorService complexity
  • ✅ No cleanup needed - daemon thread exits naturally
  • ✅ Better performance - exponential backoff reduces overhead
  • ✅ Self-healing on configuration changes

Negative

  • ⚠️ Adds background polling thread (daemon, minimal overhead, self-terminating)
  • ⚠️ Events during first ~10-20ms might be missed if DB not ready (acceptable tradeoff)

Neutral

  • Log message indicates when tables are registered
  • Uses standard Java concurrency primitives (ScheduledExecutorService)

Implementation

Tasks

Initial Implementation (2024-12-24, commit 328a8bd):

  • Add tablesRegistered volatile flag
  • Create ensureTablesRegistered() method with DB.isConnected() check
  • Move DB query from initialize() to ensureTablesRegistered()
  • Call ensureTablesRegistered() at start of doHandleEvent()
  • Add null-safe handling for indexedTablesByClient
  • Reset flag in handleSearchIndexConfigChange() for config refresh
  • ❌ FLAW DISCOVERED: Chicken-and-egg problem - lazy init never triggers

Corrected Implementation v1 (2024-12-29 - ScheduledExecutorService):

  • Add ScheduledExecutorService initScheduler field
  • Modify initialize() to try immediate registration if DB connected
  • Add background polling thread (100ms intervals, 60s timeout)
  • Rename ensureTablesRegistered() to registerDynamicTables()
  • Add shutdownScheduler() method with proper cleanup
  • Override unbindEventManager() to cleanup scheduler
  • Update handleSearchIndexConfigChange() to reinitialize properly
  • Remove ensureTablesRegistered() call from doHandleEvent()
  • Simplify null handling in doHandleEvent() with early return

Optimized Implementation v2 (2024-12-29 - Simple Thread):

  • Remove ScheduledExecutorService and concurrent imports
  • Remove initScheduler field
  • Replace initialize() with simple Thread + exponential backoff
  • Remove shutdownScheduler() method (no longer needed)
  • Simplify handleSearchIndexConfigChange() state reset
  • Simplify unbindEventManager() (no cleanup needed)
  • Exponential backoff progression: 10ms → 20ms → 40ms → 80ms → 160ms → 320ms → 500ms

Timeline

  • Initial implementation: 2024-12-24 (commit 328a8bd)
  • Problem identified: 2024-12-29
  • Corrected implementation v1 (ScheduledExecutorService): 2024-12-29
  • Optimized implementation v2 (Simple Thread): 2024-12-29

Dependencies

  • None - uses existing iDempiere infrastructure

Performance Characteristics

Exponential Backoff Progression

Starting interval: 10ms → 20ms → 40ms → 80ms → 160ms → 320ms → 500ms (max)

Scenario Delay Checks Behavior
DB already connected on startup 0ms 0 Immediate registration in initialize()
DB connects in 50ms ~10-20ms 2-3 First backoff interval catches it
DB connects in 500ms ~200-300ms ~6-7 Early backoff intervals catch it
DB connects in 5s ~2-3s ~15-20 Later backoff intervals (slower polling)
DB never connects 60s timeout ~40-50 Warning logged, graceful degradation

vs. Fixed 100ms Polling:

  • Fixed polling would need 600 checks over 60 seconds
  • Exponential backoff needs only ~40-50 checks over 60 seconds
  • 85% reduction in DB connection checks

Risks

Risk Impact Probability Mitigation
Events missed during polling Low Very Low 10ms initial interval minimizes window
DB never connects High Very Low 60s timeout with warning log
Thread doesn't exit Low Very Low Daemon thread exits with JVM
Multiple threads on rapid config changes Low Very Low tablesRegistered flag prevents duplicate work

Related

  • Related to: ADR-001 (Transaction Isolation)
  • Files Modified: SearchIndexEventHandler.java

References

  • iDempiere Core Pattern: org.compiere.util.Msg line 161 (DB.isConnected() check)
  • Java Threading: Daemon threads and exponential backoff pattern
  • OSGi Declarative Services: https://docs.osgi.org/specification/osgi.cmpn/7.0.0/service.component.html
  • Initial implementation: commit 328a8bd605fa4c8bcd935dd2d9f1d26d1b985e7e
  • Corrected v1 (ScheduledExecutorService): 2024-12-29
  • Optimized v2 (Simple Thread): 2024-12-29 (current)

Lessons Learned

1. Chicken-and-Egg in Event-Driven Systems

Event-driven lazy initialization only works if there's a guaranteed trigger. In this case, dynamic tables couldn't trigger their own registration because events only fire for registered tables.

2. Simple Solutions > Complex Frameworks

For one-time startup operations:

  • Simple Thread with exponential backoff beats ScheduledExecutorService
  • Less code, fewer moving parts, better performance
  • No cleanup needed with daemon threads

3. Exponential Backoff Benefits

  • Faster response when DB connects quickly (10ms vs 100ms first check)
  • Less CPU overhead over time (40-50 checks vs 600 checks)
  • Better user experience (minimal latency in common case)

Solution Pattern for OSGi Lazy Initialization:

  1. Fast path: Immediate registration attempt (zero-delay when ready)
  2. Slow path: Daemon thread with exponential backoff (10ms → 500ms)
  3. Timeout: Reasonable limit (60s) with graceful degradation
  4. No cleanup: Let daemon threads exit naturally

Last Updated: 2024-12-29 (Optimized v2) Review Date: 2025-06-29 (6 months from optimized implementation)