trafodion-codereview mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kevinxu021 <...@git.apache.org>
Subject [GitHub] incubator-trafodion pull request #1209: TRAFODION-2717 odbc give wrong data ...
Date Tue, 12 Sep 2017 01:34:16 GMT
Github user kevinxu021 commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/1209#discussion_r138233541
  
    --- Diff: dcs/src/main/java/org/trafodion/dcs/master/listener/ListenerWorker.java ---
    @@ -86,39 +86,43 @@ public void run() {
             DataEvent dataEvent;
         
             while(true) {
    -            // Wait for data to become available
    -            synchronized(queue) {
    -                while(queue.isEmpty()) {
    -                    try {
    -                        queue.wait();
    -                    } catch (InterruptedException e) {
    +            try {
    +                // Wait for data to become available
    +                synchronized(queue) {
    +                    while(queue.isEmpty()) {
    +                        try {
    +                            queue.wait();
    +                        } catch (InterruptedException e) {
    +                        }
                         }
    +                    dataEvent = queue.remove(0);
                     }
    -                dataEvent = queue.remove(0);
    -            }
    -            SelectionKey key = dataEvent.key;
    -            SocketChannel client = (SocketChannel) key.channel();
    -            Socket s = client.socket();
    -            ClientData clientData = (ClientData) key.attachment();
    -            ListenerService server = dataEvent.server;
    -            dataEvent.key = null;
    -            dataEvent.server = null;
    -            
    -            switch (clientData.hdr.getOperationId()){
    -                case ListenerConstants.DCS_MASTER_GETSRVRAVAILABLE:
    -                    clientData = requestGetObjectRef.processRequest(clientData, s);
    -                    break;
    -                case ListenerConstants.DCS_MASTER_CANCELQUERY:
    -                    clientData = requestCancelQuery.processRequest(clientData, s);
    -                    break;
    -                default:
    -                    clientData = requestUnknown.processRequest(clientData, s);
    -                    break;
    +                SelectionKey key = dataEvent.key;
    +                SocketChannel client = (SocketChannel) key.channel();
    +                Socket s = client.socket();
    +                ClientData clientData = (ClientData) key.attachment();
    +                ListenerService server = dataEvent.server;
    +                dataEvent.key = null;
    +                dataEvent.server = null;
    +
    +                switch (clientData.hdr.getOperationId()){
    +                    case ListenerConstants.DCS_MASTER_GETSRVRAVAILABLE:
    +                        clientData = requestGetObjectRef.processRequest(clientData, s);
    +                        break;
    +                    case ListenerConstants.DCS_MASTER_CANCELQUERY:
    +                        clientData = requestCancelQuery.processRequest(clientData, s);
    +                        break;
    +                    default:
    +                        clientData = requestUnknown.processRequest(clientData, s);
    +                        break;
    +                }
    +                // Return to sender
    +                int requestReply = clientData.requestReply;
    +                key.attach(clientData);
    +                server.send(new PendingRequest(key, requestReply));
    +            } catch (Exception e){
    +                LOG.error("Unexpected Exception", e);
    --- End diff --
    
    This is an issue for memory allocation failed, it doesn't mean the memory has been ran
out. I dont think it is reasonable to exit the process instead of returning back the issue
to the client. By the way, even we have socket timeout, the real issue will be hidden. whatever
the issue is on server-side, you will always get socket timeout for every client request.
Can customer really accept the behavior? Again, the outside CATCH is to avoid process exiting
because of a very particular bad request. If it is a real OUT OF MEMORY´╝î i mean run out
the memory, absolutely the process will be exiting.  More, OutOfMemoryError is an ERROR which
not belongs to exception and will not be covered by EXCEPTION block. In another word, the
process will exit.


---

Mime
View raw message