Details

    • Type: Improvement Improvement
    • Status: Open
    • Priority: Trivial Trivial
    • Resolution: Unresolved
    • Affects Version/s: 1.8.2 Beta
    • Fix Version/s: None
    • Component/s: Enterprise Push Server
    • Labels:
      None
    • Environment:
      ICEFaces Enterprise push server, Websphere application server 6.1

      Description

      When we were running with WebSphere configured in High Availability mode (namely, with the cluster as a member of the bus), the documents state that the messaging engine will run on one node of the cluster. If that node should fail, the messaging engine will shut down, and restart on one of the other servers in the cluster.

      When this happens, the MessageReceiver's blocking read of the JMS topic will throw an exception. Ideally, the EPS code would be able to release these JMS resources, and reconnect/reobtain all connections to the topicConnectionFactory and the topic itself and resume operations. This doesn't happen currently because, while the code's reconnectTask repeatedly attempts to tear down and reconnect to JMS resources, exceptions thrown in the shutdown of said resources (in WebSphere in particular) prevent the teardown task from changing state. In words, we could only try to tear down these resources and if they are closed or shutting them down throws an exception, null as many references as possible, and move on.

      It turns out that you can configure the JMS resources for high scalability, meaning you can define a message bus for each node and pin it to a particular node in the cluster. This eliminates the need for this code improvement since the eps clients running on any node don't notice when the message bus on another node shuts down. This Jira is just a record of the changes I made to address the first problem.

      This is the new run() method of the TearDownTask class in the DefaultMessageService

       public void run() {
                  LOG.debug("Executing Tear Down task...");
                  try {
                      tearDownMessageServiceClient();
                  } catch (Exception exception) {
                      System.out.println("Exception caught tearing down client: " + exception);
                  }
                  if (scheduledFuture != null) {
                      scheduledFuture.cancel(false);
                      scheduledFuture = null;
                  }
                  succeeded = true;
                  synchronized (stateLock) {
                      currentState = STATE_TEAR_DOWN_DONE;
                      if (LOG.isDebugEnabled()) {
                          LOG.debug("Current State: TEAR DOWN DONE");
                      }
                      if (requestedState == STATE_CLOSED) {
                          close();
                      }
                  }
                  LOG.debug("Executing Tear Down task... (succeeded: [" + succeeded + "])");
              }


      This now surrounds the invocation of tearDownMessageServiceClient(); with try-catch rather than try-catch surrounding the entire block, so the state cleanup code can always execute.

      Second change was in the close() method of the JMSAdapter:


          /**
           * Attempt to close all JMS resources once, but keep track of only the
           * first exception if there were more than one.
           * @throws MessageServiceException wrapping the original exception
           */
          public void close()
                  throws MessageServiceException {
      // synchronized (topicPublisherMap) {
      // synchronized (topicPublisherMap) {

              Exception firstException = null;

              if (!topicPublisherMap.isEmpty()) {
                  Iterator _jmsPublisherConnections =
                          topicPublisherMap.values().iterator();
                  while (_jmsPublisherConnections.hasNext()) {
                      try {
                          // throws JMSException.
                          ((JMSPublisherConnection)
                                  _jmsPublisherConnections.next()).close();

                      } catch (JMSException jms) {
                          if (firstException == null) {
                              firstException = jms;
                          }
                      }
                  }
              }
              if (!topicSubscriberMap.isEmpty()) {
                  Iterator _jmsSubscriberConnections =
                          topicSubscriberMap.values().iterator();
                  while (_jmsSubscriberConnections.hasNext()) {
                      try {
                          // throws JMSException.
                          ((JMSSubscriberConnection)
                                  _jmsSubscriberConnections.next()).close();
                      } catch (JMSException jms) {
                          if (firstException == null) {
                              firstException = jms;
                          }
                      }
                  }
              }
              try {
                  if (initialContext != null) {
                      initialContext.close();
                  }
              } catch (NamingException nm) {
                  if (firstException == null) {
                      firstException = nm;
                  }
              }
              topicPublisherMap.clear();
              topicSubscriberMap.clear();
              topicConnectionFactory = null;
              initialContext = null;

              if (firstException != null) {
                  throw new MessageServiceException(firstException);
              }
          }


      Which again clears up the resources if an exception is thrown and will attempt to close each of the resources at least once.

        Activity

        Hide
        Greg Dick added a comment -

        Assign to Jack

        Show
        Greg Dick added a comment - Assign to Jack
        Hide
        Greg Dick added a comment -

        Changing to trivial

        Show
        Greg Dick added a comment - Changing to trivial
        Hide
        Patrick Corless added a comment -

        Not targeted yet but though someone should own it.

        Show
        Patrick Corless added a comment - Not targeted yet but though someone should own it.

          People

          • Assignee:
            Jack Van Ooststroom
            Reporter:
            Greg Dick
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated: