SAP / node-rfc

Asynchronous, non-blocking SAP NW RFC SDK bindings for Node.js
Apache License 2.0
252 stars 73 forks source link

Is there a way to make the rfc call Async? #29

Closed doronoded closed 6 years ago

doronoded commented 7 years ago

When calling invoke(), although one of the parameters is a callback function, the call itself is synchronous. The javascript thread is stuck untill the callback function is being called. Kind of creates a problematic situation where the node process cannot serve any other request while an rfc call is in process.

Is there a way to call the invoke function Async? Doron

bsrdjan commented 7 years ago

The node-rfc utilizes NAN and libuv library to ensure the asynchronous behavior and the javascript thread should not stuck. Did you test and can you share the example to reproduce?

doronoded commented 7 years ago

Hi,

Thank you very much for the swift response! I have tested it on Windows. The test is very simple. Simply call an rfc (via the invoke function), write to the console right after the call, and inside the callback function. Since it is synchronous, the console output from the callback will be wriitten before the writing after the function call.

I will test this on linux as well and keep you posted.

Doron

בתאריך 23 ביוני 2017 11:51,‏ "Srdjan Boskovic" notifications@github.com כתב:

The node-rfc utilizes NAN https://github.com/nodejs/nan and libuv https://github.com/libuv/libuv library to ensure the asynchronous behavior and the javascript thread should not stuck. Did you test and can you share the example to reproduce?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/SAP/node-rfc/issues/29#issuecomment-310610330, or mute the thread https://github.com/notifications/unsubscribe-auth/ADP71bp9eJLdQqPA60_DtnW6a8zexGZwks5sG3x7gaJpZM4OB5Qt .

bsrdjan commented 7 years ago

There is a async test unit test, covering exactly this case - async invoke. I tested on both platforms, using the nodejs 6.11.0 and the master branch and all tests passed. Could you please share the test code that I could run on a local test system here, to try to reproduce?

doronoded commented 7 years ago

OK, What I wrote you previously is not correct. The order of the commands is as it should be, but still, the node process freezes. Here is a proposal of a unit test that will prove my point:

it('async test', function(done) {
    var asyncRes = undefined;
    var anotherAsyncFunctionToCompare = function(){setTimeout(function(){console.log("anotherAsyncFunctionToCompare after 2 seconds.")}, 2000)}

    var t = Date.now();

    client.invoke('STFC_CONNECTION', { REQUTEXT: 'Hello SAP!' },  //async function 1,
      function(err, res) {
        should.not.exist(err);
        should.exist(res);
        res.should.be.an.Object;
        res.should.have.property('ECHOTEXT');
        res.ECHOTEXT.should.startWith('Hello SAP!');
        asyncRes = res;
        //done();
      });
    should.not.exist(asyncRes);

    var timeItTookForFirstAsync = Date.now() - t;
    console.log("look how much time took to print this line after the 'invoke' function: " + timeItTookForFirstAsync);

    //lets compare to another async function that takes 2 seconds:
    t = Date.now();
    anotherAsyncFunctionToCompare(); //async function 2
    var timeItTookForSecondAsync = Date.now() - t;
    console.log("look how much time took to print this line after the second async call: " + timeItTookForSecondAsync);

    (timeItTookForFirstAsync - timeItTookForSecondAsync).should.be.below(5);

done();
  });
bsrdjan commented 7 years ago

The invoke method has internally three phases: packing nodejs client input parameters into RFC container, sending the RFC container to backend, receiving the updated container from backend and unpacking the result into nodejs client output parameters. The first phase is blocking (here the metadata retrieval happens) and considered uncritical because the input parameters handover/takeover takes much less time than the RFC call itself. Also from the node-rfc user's perspective, it is more convenient and elegant to write programs with one invoke/callback, rather than parameter-handover/callback, related to subsequent invoke/callback.

Just for the reference, following script illustrates the behaviour, showing a bit more milliseconds tick after each call of new RFC function, but the lock is immediately released after placing the call, not waiting for the result from backend:

var client = new rfc.Client(abapSystem);

console.log('rfcClient:', client.getVersion());

client.connect(function (err){
    if (err){
        console.log('Error in the login process: ' + err);
        return;
    }
    console.log('connected to rfc client');
    var t = Date.now();
    var counter = 0;

    console.log('calling 1: BAPI_USER_GET_DETAIL', counter, Date.now() - t);
    client.invoke('BAPI_USER_GET_DETAIL', {USERNAME: 'DEMO'},
      function(err, res) {
          if (err) {
              console.log('Error calling 1 BAPI_USER_GET_DETAIL: ', err);
          }
          else {
              console.log('RFC call 1 BAPI_USER_GET_DETAIL success!', counter++, Date.now() - t);
          }
      });

    console.log('calling 2: BAPI_USER_GET_DETAIL', counter, Date.now() - t);
    client.invoke('BAPI_USER_GET_DETAIL', {USERNAME: 'DEMO'},
        function(err, res) {
            if (err) {
                console.log('Error calling 2 BAPI_USER_GET_DETAIL: ', err);
            }
            else {
                counter ++
                console.log('RFC call 2 BAPI_USER_GET_DETAIL success!', counter++, Date.now() - t);
            }
        });

    console.log('calling 3: BAPI_USER_GET_DETAIL', counter, Date.now() - t);
    client.invoke('BAPI_USER_GET_DETAIL', {USERNAME: 'DEMO'},
        function(err, res) {
            if (err) {
                console.log('Error calling 3 BAPI_USER_GET_DETAIL: ', err);
            }
            else {
                console.log('RFC call 3 BAPI_USER_GET_DETAIL success!', counter++, Date.now() - t);
            }
        });

    console.log('calling 4: BAPI_PO_GETDETAIL1', counter, Date.now() - t);
    client.invoke('BAPI_PO_GETDETAIL1', {PURCHASEORDER: '4500000001'},
        function(err, res) {
            if (err) {
                console.log('Error calling 3 BAPI_PO_GETDETAIL1: ', err);
            }
            else {
                console.log('RFC call 4 BAPI_PO_GETDETAIL1 success!', counter++, Date.now() - t);
            }

        });

    console.log('calling 5: BAPI_PO_GETDETAIL1', counter, Date.now() - t);
    client.invoke('BAPI_PO_GETDETAIL1', {PURCHASEORDER: '4500000001'},
        function(err, res) {
            if (err) {
                console.log('Error calling 3 BAPI_PO_GETDETAIL1: ', err);
            }
            else {
                console.log('RFC call 5 BAPI_PO_GETDETAIL1 success!', counter++, Date.now() - t);
            }
        });

    console.log('calling 6: BAPI_PO_GETDETAIL1', counter, Date.now() - t);
    client.invoke('BAPI_PO_GETDETAIL1', {PURCHASEORDER: '4500000001'},
        function(err, res) {
            if (err) {
                console.log('Error calling 3 BAPI_PO_GETDETAIL1: ', err);
            }
            else {
                console.log('RFC call 6 BAPI_PO_GETDETAIL1 success!', counter++, Date.now() - t);
            }
        });
});

Lines 2 and 5 illustrate the blocking caused by the metadata retrieval:

calling 1: BAPI_USER_GET_DETAIL 0 0
calling 2: BAPI_USER_GET_DETAIL 0 1497
calling 3: BAPI_USER_GET_DETAIL 0 1575
calling 4: BAPI_PO_GETDETAIL1 0 1589
calling 5: BAPI_PO_GETDETAIL1 0 3267
calling 6: BAPI_PO_GETDETAIL1 0 3298
RFC call 1 BAPI_USER_GET_DETAIL success! 0 3322
RFC call 2 BAPI_USER_GET_DETAIL success! 2 3322
RFC call 3 BAPI_USER_GET_DETAIL success! 3 3323
RFC call 4 BAPI_PO_GETDETAIL1 success! 4 3324
RFC call 5 BAPI_PO_GETDETAIL1 success! 5 3324
RFC call 6 BAPI_PO_GETDETAIL1 success! 6 3325

No plans/reasons to change this for the time being.

doronoded commented 7 years ago

Well, we are very sorry to hear that there are no plans to change the blocking of the node process.. Imagine the node process serves several requests for several users, each causes it to freeze for few seconds. Especially consider the fact we can't use the rfc client object for more than one user (due to different credentials the rfc client object has in it's internals). This means this is not scaleable at all.

I hope you will change your mind, and do try to make the call fully async without blocking the node process, and in addition I think it will also help to separate the blocking function from the non blocking function and it's dependence in credentials. Meaning, if we could run the first long blocking call for all users, and then create from it instances of rfc clients with different credentials, but with the meta data and structures already cached, it would be a very good optimization and performance improvement.

bsrdjan commented 7 years ago

Behaviour documented

bsrdjan commented 6 years ago

@doronoded,

the RFC module @Berdmanfolk used for #29 testing, looks very nice for testing async features as well.

It is widely available and can run practically arbitrary long, by varying start/end dates for example.

Here one handy example based on that RFC, for testing node-rfc non-blocking async operations.

If any further concerns or questions about node-rfc blocking, non-blocking or scalability, please feel free to test in your systems or reopen this issue.


let client = new rfc.Client(connParams);

console.log(Date.now(), 'connect');
client.connect(function (err) {
    console.log(Date.now(), 'connect callback');
    if (err) { // check for login/connection errors
       return console.error('could not connect to server', err);
    }
    console.log(Date.now(), 'invoke');
    // setTimeout(()=>{console.log('async callback')}, 2000);
    client.invoke('SWNC_READ_SNAPSHOT', {READ_TIMEZONE: 'UTC', READ_START_DATE: '20180422', READ_START_TIME: '000000', READ_END_DATE: '20180424', READ_END_TIME: '235959', TIME_RESOLUTION: 60}, function (err, res) {
        if (err) {
            console.log(Date.now(), 'invoke callback error',err)
        } else {
            client.close();
            console.log(Date.now(), 'invoke callback result', res.RECORDS_READ);
        }
    });
});

console.log(Date.now(), 'main');

Console output:

1524476047319 'connect'
1524476047321 'main'
1524476047391 'connect callback'
1524476047391 'invoke'
1524476048709 'invoke callback error' { Error:  Number:000
  code: 5,
  key: 'NO_DATA',
  abapMsgClass: '',
  abapMsgType: '',
  abapMsgNumber: '000',
  abapMsgV1: '',
  abapMsgV2: '',
  abapMsgV3: '',
  abapMsgV4: '' }

With async callback just before invoke uncommented and with longer running RFC, the result is:

1524477912303 'connect'
1524477912305 'main'
1524477912373 'connect callback'
1524477912373 'invoke'
async callback
1524477979974 'invoke callback result' 43614