ICEpush
  1. ICEpush
  2. PUSH-26

Log a warning when pushing to a non-existent group

          Details

          • Type: Improvement Improvement
          • Status: Closed
          • Priority: Major Major
          • Resolution: Fixed
          • Affects Version/s: 2.0-Alpha2
          • Fix Version/s: 2.0-Alpha3
          • Component/s: Push Library
          • Labels:
            None
          • Environment:
            n/a

            Description

            It will be very useful in development to log a warning when PushContext.push(String group) is called for a non-existent group. Currently, the group name is treated as a single pushId if the group does not exist, but we could still check if that pushId exists, and if not, log a warning. This will improve the monitoring capabilities of an ICEpush application.

            Note that in a clustered environment a node may have no knowledge of a particular group, and a warning might be erroneously logged.

              Activity

              Ascending order - Click to sort in descending order
              Philip Breau created issue -
              Deryk Sinotte made changes -
              Field Original Value New Value
              Summary Log a warning when pusing to a non-existent group Log a warning when pushing to a non-existent group
              Fix Version/s 1.0-Alpha2 [ 10223 ]
              Assignee Priority P2
              Affects Version/s 1.0-Alpha2 [ 10223 ]
              Assignee Mircea Toma [ mircea.toma ]
              Mircea Toma made changes -
              Affects Version/s 1.0-Alpha3 [ 10224 ]
              Affects Version/s 1.0-Alpha2 [ 10223 ]
              Mircea Toma made changes -
              Fix Version/s 1.0-Alpha3 [ 10224 ]
              Fix Version/s 1.0-Alpha2 [ 10223 ]
              Affects Version/s 1.0-Alpha2 [ 10223 ]
              Affects Version/s 1.0-Alpha3 [ 10224 ]
              Repository Revision Date User Message
              ICEsoft Public SVN Repository #20743 Mon Feb 22 16:05:31 MST 2010 mircea.toma PUSH-26 Keep track of registered pushIDs. Log warning for rogue pushID.
              Files Changed
              Commit graph MODIFY /scratchpads/snowplow/core/src/main/java/org/icepush/PushContext.java
              Commit graph ADD /scratchpads/snowplow/core/src/main/java/org/icepush/VetoedSet.java
              Hide
              Permalink
              Mircea Toma added a comment - - edited

              Keep track of registered pushIDs to log warning when rogue pushIDs are used. To avoid extensive group scanning on each notification call a special Set implementation is used. Its implementation keeps track of the number of times a pushId is used to avoid removing it from the set of registered pushIds if multiple groups own the same pushId.

              Show
              Mircea Toma added a comment - - edited Keep track of registered pushIDs to log warning when rogue pushIDs are used. To avoid extensive group scanning on each notification call a special Set implementation is used. Its implementation keeps track of the number of times a pushId is used to avoid removing it from the set of registered pushIds if multiple groups own the same pushId.
              Mircea Toma made changes -
              Status Open [ 1 ] Resolved [ 5 ]
              Resolution Fixed [ 1 ]
              Hide
              Permalink
              Ted Goddard added a comment -

              This feature may have disappeared with the refactoring to include cluster support.

              The other concern, though, is that the logging may be unbounded. For instance, closing a window in auction results in:

              Mar 15, 2010 4:51:34 PM org.icepush.PushContext push
              WARNING: 'auction' pushId does not belong to a group.
              Mar 15, 2010 4:51:35 PM org.icepush.PushContext push
              WARNING: 'auction' pushId does not belong to a group.
              Mar 15, 2010 4:51:36 PM org.icepush.PushContext push
              WARNING: 'auction' pushId does not belong to a group.

              To avoid storing the name of every failed group, it might be sufficient to suppress the logging just for the last failed group.

              Show
              Ted Goddard added a comment - This feature may have disappeared with the refactoring to include cluster support. The other concern, though, is that the logging may be unbounded. For instance, closing a window in auction results in: Mar 15, 2010 4:51:34 PM org.icepush.PushContext push WARNING: 'auction' pushId does not belong to a group. Mar 15, 2010 4:51:35 PM org.icepush.PushContext push WARNING: 'auction' pushId does not belong to a group. Mar 15, 2010 4:51:36 PM org.icepush.PushContext push WARNING: 'auction' pushId does not belong to a group. To avoid storing the name of every failed group, it might be sufficient to suppress the logging just for the last failed group.
              Philip Breau made changes -
              Comment [ Is that warning from auction occurring because of the older icepush.jar version currently used in Glimmer? Can't we just check in the PushGroupManager impl like so:
              ]
              Hide
              Permalink
              Philip Breau added a comment -

              Is that warning being logged in auction because of the older icepush.jar version currently used in Glimmer which was checking pushIDs directly (when you previous could push to id's directly)? Couldn't we just move the logging to LocalPushGroupManager like so. The warning could just be ignored for clustered deployments.

              Index: /home/philip/workspace/ICEpush/core/src/main/java/org/icepush/LocalPushGroupManager.java
              ===================================================================
              — /home/philip/workspace/ICEpush/core/src/main/java/org/icepush/LocalPushGroupManager.java (revision 20901)
              +++ /home/philip/workspace/ICEpush/core/src/main/java/org/icepush/LocalPushGroupManager.java (working copy)
              @@ -94,7 +94,10 @@
              if (LOGGER.isLoggable(Level.FINEST))

              { LOGGER.log(Level.FINEST, "Push notification triggered for '" + groupName + "' group."); }
              • outboundNotifier.notifyObservers(groupMap.get(groupName).getPushIDs());
                + if( ! groupMap.containsKey(groupName))
                + LOGGER.warning(..);
                + else
                + outboundNotifier.notifyObservers(groupMap.get(groupName).getPushIDs());
                }
                }
              Show
              Philip Breau added a comment - Is that warning being logged in auction because of the older icepush.jar version currently used in Glimmer which was checking pushIDs directly (when you previous could push to id's directly)? Couldn't we just move the logging to LocalPushGroupManager like so. The warning could just be ignored for clustered deployments. Index: /home/philip/workspace/ICEpush/core/src/main/java/org/icepush/LocalPushGroupManager.java =================================================================== — /home/philip/workspace/ICEpush/core/src/main/java/org/icepush/LocalPushGroupManager.java (revision 20901) +++ /home/philip/workspace/ICEpush/core/src/main/java/org/icepush/LocalPushGroupManager.java (working copy) @@ -94,7 +94,10 @@ if (LOGGER.isLoggable(Level.FINEST)) { LOGGER.log(Level.FINEST, "Push notification triggered for '" + groupName + "' group."); } outboundNotifier.notifyObservers(groupMap.get(groupName).getPushIDs()); + if( ! groupMap.containsKey(groupName)) + LOGGER.warning(..); + else + outboundNotifier.notifyObservers(groupMap.get(groupName).getPushIDs()); } }
              Ken Fyten made changes -
              Status Resolved [ 5 ] Closed [ 6 ]
              Assignee Priority P2

                People

                • Assignee:
                  Mircea Toma
                  Reporter:
                  Philip Breau
                • Votes:
                  0 Vote for this issue
                  Watchers:
                  0 Start watching this issue

                  Dates

                  • Created:
                    Updated:
                    Resolved: