You are viewing a plain text version of this content. The canonical link for it is here.
Posted to codereview@trafodion.apache.org by nonstop-qfchen <gi...@git.apache.org> on 2016/01/06 17:09:06 UTC

[GitHub] incubator-trafodion pull request: fix JIRA-1739 to aggressively al...

GitHub user nonstop-qfchen opened a pull request:

    https://github.com/apache/incubator-trafodion/pull/250

    fix JIRA-1739 to aggressively allocate ESPs.

    Please review the fix. Thanks 

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/nonstop-qfchen/incubator-trafodion portDoP

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-trafodion/pull/250.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #250
    
----
commit 2fc4eba602b18de74b4ca4e700b3a8987090e310
Author: Qifan Chen <qf...@adev04.esgyn.com>
Date:   2016-01-06T16:07:31Z

    fix JIRA-1739 to aggressively allocate ESPs.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: fix JIRA-1739 to aggressively al...

Posted by zellerh <gi...@git.apache.org>.
Github user zellerh commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/250#discussion_r49023709
  
    --- Diff: core/sql/sqlcomp/nadefaults.cpp ---
    @@ -5914,6 +5866,73 @@ enum DefaultConstants NADefaults::validateAndInsert(const char *attrName,
     
     } // NADefaults::validateAndInsert()
     
    +float NADefaults::computeNumESPsPerCore(NABoolean aggressive)
    +{
    +   #define DEFAULT_ESPS_PER_NODE 2   // for conservation allocation
    +   #define DEFAULT_ESPS_PER_CORE 0.5 // for aggressive allocation
    +
    +     // Make sure the gpClusterInfo points at an NAClusterLinux object.
    +     // In osim simulation mode, the pointer can point at a NAClusterNSK
    +     // object, for which the method numTSEsForPOS() is not defined.
    +   NAClusterInfoLinux* gpLinux = dynamic_cast<NAClusterInfoLinux*>(gpClusterInfo);
    +   assert(gpLinux);		
    +
    +   // cores per node
    +   Lng32 coresPerNode = gpClusterInfo->numberOfCpusPerSMP();
    +
    +   if ( aggressive ) {
    +      float totalMemory = gpLinux->totalMemoryAvailable(); // per Node, in KB
    +      totalMemory /= 1024; // per Node, in GB
    +      totalMemory /= coresPerNode ; // per core, in GB
    +      totalMemory /= 2; // per core, 2GB per ESP
    --- End diff --
    
    Should this be another CQD?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: fix JIRA-1739 to aggressively al...

Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/250#discussion_r49126763
  
    --- Diff: core/sql/sqlcomp/nadefaults.cpp ---
    @@ -4333,59 +4336,8 @@ void NADefaults::updateSystemParameters(NABoolean reInit)
     
         case MAX_ESPS_PER_CPU_PER_OP:
           {
    -        // set 2 ESPs per node, as a starting point.
    -        #define DEFAULT_ESPS_PER_NODE 2
    -
    -        Lng32 numESPsPerNode = DEFAULT_ESPS_PER_NODE;
    -        Lng32 coresPerNode = 1;        
    -          // Make sure the gpClusterInfo points at an NAClusterLinux object.
    -          // In osim simulation mode, the pointer can point at a NAClusterNSK
    -          // object, for which the method numTSEsForPOS() is not defined.
    -        NAClusterInfoLinux* gpLinux = dynamic_cast<NAClusterInfoLinux*>(gpClusterInfo);
    -
    -          // number of POS TSE
    -        Lng32 numTSEsPerCluster = gpLinux->numTSEsForPOS();
    -
    -          // cluster nodes
    -        Lng32 nodesdPerCluster = gpClusterInfo->getTotalNumberOfCPUs();
    -
    -          // TSEs per node
    -        Lng32 TSEsPerNode = numTSEsPerCluster/nodesdPerCluster;
    -
    -          // cores per node
    -        coresPerNode = gpClusterInfo->numberOfCpusPerSMP();
    -
    -          // For Linux/nt, we conservatively allocate ESPs per node as follows
    -          // - 1 ESP per 2 cpu cores if cores are equal or less than TSEs
    -          // - 1 ESP per TSE if number of cores is more than double the TSEs
    -          // - 1 ESP per 2 TSEs if cores are more than TSEs but less than double the TSEs
    -          // - 1 ESP per node. Only possible on NT or workstations
    -          //      - number of cores less than TSEs and there are 1 or 2 cpur cores per node
    -          //      - number of TSEs is less than cpu cores and there 1 or 2 TSEs per node.
    -          //        This case is probable if virtual nodes are used
    -
    -          // TSEsPerNode is 0 for arkcmps started by the seapilot universal comsumers
    -          // in this case we only consider cpu cores
    -        if ((coresPerNode <= TSEsPerNode) || (TSEsPerNode == 0))
    -        {
    -            if (coresPerNode > 1)
    -                numESPsPerNode = DEFAULT_ESPS_PER_NODE; 
    -        }
    -        else if (coresPerNode > (TSEsPerNode*2))
    -        {
    -             numESPsPerNode = TSEsPerNode;
    -        }
    -        else if (TSEsPerNode > 1)
    -        {
    -             numESPsPerNode = TSEsPerNode/2;
    -        }
    -        else // not really needed since numESPsPerNode is set to 1 from above
    -        {
    -             numESPsPerNode = DEFAULT_ESPS_PER_NODE;
    -        }
    -        
    -
    -        ftoa_((float)(numESPsPerNode)/(float)(coresPerNode), valuestr);
    +        float espsPerCore = computeNumESPsPerCore(FALSE);
    --- End diff --
    
    OK, I think I get it now. If CQD AGGRESSIVE_ESP_ALLOCATION_PER_CORE is set to 'ON', then under the covers we change the value of the default MAX_ESPS_PER_CPU_PER_OP. So, this new CQD is in a sense a meta-CQD.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: fix JIRA-1739 to aggressively al...

Posted by zellerh <gi...@git.apache.org>.
Github user zellerh commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/250#discussion_r49024774
  
    --- Diff: core/sql/sqlcomp/nadefaults.cpp ---
    @@ -5914,6 +5866,73 @@ enum DefaultConstants NADefaults::validateAndInsert(const char *attrName,
     
     } // NADefaults::validateAndInsert()
     
    +float NADefaults::computeNumESPsPerCore(NABoolean aggressive)
    +{
    +   #define DEFAULT_ESPS_PER_NODE 2   // for conservation allocation
    +   #define DEFAULT_ESPS_PER_CORE 0.5 // for aggressive allocation
    +
    +     // Make sure the gpClusterInfo points at an NAClusterLinux object.
    +     // In osim simulation mode, the pointer can point at a NAClusterNSK
    +     // object, for which the method numTSEsForPOS() is not defined.
    +   NAClusterInfoLinux* gpLinux = dynamic_cast<NAClusterInfoLinux*>(gpClusterInfo);
    +   assert(gpLinux);		
    +
    +   // cores per node
    +   Lng32 coresPerNode = gpClusterInfo->numberOfCpusPerSMP();
    +
    +   if ( aggressive ) {
    +      float totalMemory = gpLinux->totalMemoryAvailable(); // per Node, in KB
    --- End diff --
    
    Allocating one ESP per 2 GB seems a bit arbitrary to me. So, rather than asking the user to set the AGGRESSIVE_ESP_ALLOCATION_PER_CORE to ON (which will eventually be used, I guess), should we just ask them to pick an appropriate value for MAX_ESPS_PER_CPU_PER_OP instead?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: fix JIRA-1739 to aggressively al...

Posted by zellerh <gi...@git.apache.org>.
Github user zellerh commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/250#discussion_r49035086
  
    --- Diff: core/sql/sqlcomp/nadefaults.cpp ---
    @@ -5914,6 +5866,73 @@ enum DefaultConstants NADefaults::validateAndInsert(const char *attrName,
     
     } // NADefaults::validateAndInsert()
     
    +float NADefaults::computeNumESPsPerCore(NABoolean aggressive)
    +{
    +   #define DEFAULT_ESPS_PER_NODE 2   // for conservation allocation
    +   #define DEFAULT_ESPS_PER_CORE 0.5 // for aggressive allocation
    +
    +     // Make sure the gpClusterInfo points at an NAClusterLinux object.
    +     // In osim simulation mode, the pointer can point at a NAClusterNSK
    +     // object, for which the method numTSEsForPOS() is not defined.
    +   NAClusterInfoLinux* gpLinux = dynamic_cast<NAClusterInfoLinux*>(gpClusterInfo);
    +   assert(gpLinux);		
    +
    +   // cores per node
    +   Lng32 coresPerNode = gpClusterInfo->numberOfCpusPerSMP();
    +
    +   if ( aggressive ) {
    +      float totalMemory = gpLinux->totalMemoryAvailable(); // per Node, in KB
    +      totalMemory /= 1024; // per Node, in GB
    +      totalMemory /= coresPerNode ; // per core, in GB
    +      totalMemory /= 2; // per core, 2GB per ESP
    +      return MINOF(DEFAULT_ESPS_PER_CORE, totalMemory);
    +   }
    +
    +   Lng32 numESPsPerNode = DEFAULT_ESPS_PER_NODE;
    +
    +     // number of POS TSE
    +   Lng32 numTSEsPerCluster = gpLinux->numTSEsForPOS();
    --- End diff --
    
    My vote is for getting rid of the code, it's confusing. As you say, it's probably not very useful anyway. Why don't we just use MAX_ESPS_PER_CPU_PER_OP (default value of 0.5) for the non-aggressive method?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: fix JIRA-1739 to aggressively al...

Posted by nonstop-qfchen <gi...@git.apache.org>.
Github user nonstop-qfchen commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/250#discussion_r49027039
  
    --- Diff: core/sql/sqlcomp/nadefaults.cpp ---
    @@ -5914,6 +5866,73 @@ enum DefaultConstants NADefaults::validateAndInsert(const char *attrName,
     
     } // NADefaults::validateAndInsert()
     
    +float NADefaults::computeNumESPsPerCore(NABoolean aggressive)
    +{
    +   #define DEFAULT_ESPS_PER_NODE 2   // for conservation allocation
    +   #define DEFAULT_ESPS_PER_CORE 0.5 // for aggressive allocation
    +
    +     // Make sure the gpClusterInfo points at an NAClusterLinux object.
    +     // In osim simulation mode, the pointer can point at a NAClusterNSK
    +     // object, for which the method numTSEsForPOS() is not defined.
    +   NAClusterInfoLinux* gpLinux = dynamic_cast<NAClusterInfoLinux*>(gpClusterInfo);
    +   assert(gpLinux);		
    +
    +   // cores per node
    +   Lng32 coresPerNode = gpClusterInfo->numberOfCpusPerSMP();
    +
    +   if ( aggressive ) {
    +      float totalMemory = gpLinux->totalMemoryAvailable(); // per Node, in KB
    +      totalMemory /= 1024; // per Node, in GB
    +      totalMemory /= coresPerNode ; // per core, in GB
    +      totalMemory /= 2; // per core, 2GB per ESP
    +      return MINOF(DEFAULT_ESPS_PER_CORE, totalMemory);
    +   }
    +
    +   Lng32 numESPsPerNode = DEFAULT_ESPS_PER_NODE;
    +
    +     // number of POS TSE
    +   Lng32 numTSEsPerCluster = gpLinux->numTSEsForPOS();
    --- End diff --
    
    Probably wait? I am not sure if TSE-style storage is possible in the
    future.
    
    On Wed, Jan 6, 2016 at 5:21 PM, Hans Zeller <no...@github.com>
    wrote:
    
    > In core/sql/sqlcomp/nadefaults.cpp
    > <https://github.com/apache/incubator-trafodion/pull/250#discussion_r49023989>
    > :
    >
    > > +
    > > +   // cores per node
    > > +   Lng32 coresPerNode = gpClusterInfo->numberOfCpusPerSMP();
    > > +
    > > +   if ( aggressive ) {
    > > +      float totalMemory = gpLinux->totalMemoryAvailable(); // per Node, in KB
    > > +      totalMemory /= 1024; // per Node, in GB
    > > +      totalMemory /= coresPerNode ; // per core, in GB
    > > +      totalMemory /= 2; // per core, 2GB per ESP
    > > +      return MINOF(DEFAULT_ESPS_PER_CORE, totalMemory);
    > > +   }
    > > +
    > > +   Lng32 numESPsPerNode = DEFAULT_ESPS_PER_NODE;
    > > +
    > > +     // number of POS TSE
    > > +   Lng32 numTSEsPerCluster = gpLinux->numTSEsForPOS();
    >
    > Since there are no TSEs in Trafodion, can we get rid of this code?
    >
    > —
    > Reply to this email directly or view it on GitHub
    > <https://github.com/apache/incubator-trafodion/pull/250/files#r49023989>.
    >
    
    
    
    -- 
    Regards, --Qifan



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: fix JIRA-1739 to aggressively al...

Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/250#discussion_r49092140
  
    --- Diff: core/sql/sqlcomp/nadefaults.cpp ---
    @@ -4333,59 +4336,8 @@ void NADefaults::updateSystemParameters(NABoolean reInit)
     
         case MAX_ESPS_PER_CPU_PER_OP:
           {
    -        // set 2 ESPs per node, as a starting point.
    -        #define DEFAULT_ESPS_PER_NODE 2
    -
    -        Lng32 numESPsPerNode = DEFAULT_ESPS_PER_NODE;
    -        Lng32 coresPerNode = 1;        
    -          // Make sure the gpClusterInfo points at an NAClusterLinux object.
    -          // In osim simulation mode, the pointer can point at a NAClusterNSK
    -          // object, for which the method numTSEsForPOS() is not defined.
    -        NAClusterInfoLinux* gpLinux = dynamic_cast<NAClusterInfoLinux*>(gpClusterInfo);
    -
    -          // number of POS TSE
    -        Lng32 numTSEsPerCluster = gpLinux->numTSEsForPOS();
    -
    -          // cluster nodes
    -        Lng32 nodesdPerCluster = gpClusterInfo->getTotalNumberOfCPUs();
    -
    -          // TSEs per node
    -        Lng32 TSEsPerNode = numTSEsPerCluster/nodesdPerCluster;
    -
    -          // cores per node
    -        coresPerNode = gpClusterInfo->numberOfCpusPerSMP();
    -
    -          // For Linux/nt, we conservatively allocate ESPs per node as follows
    -          // - 1 ESP per 2 cpu cores if cores are equal or less than TSEs
    -          // - 1 ESP per TSE if number of cores is more than double the TSEs
    -          // - 1 ESP per 2 TSEs if cores are more than TSEs but less than double the TSEs
    -          // - 1 ESP per node. Only possible on NT or workstations
    -          //      - number of cores less than TSEs and there are 1 or 2 cpur cores per node
    -          //      - number of TSEs is less than cpu cores and there 1 or 2 TSEs per node.
    -          //        This case is probable if virtual nodes are used
    -
    -          // TSEsPerNode is 0 for arkcmps started by the seapilot universal comsumers
    -          // in this case we only consider cpu cores
    -        if ((coresPerNode <= TSEsPerNode) || (TSEsPerNode == 0))
    -        {
    -            if (coresPerNode > 1)
    -                numESPsPerNode = DEFAULT_ESPS_PER_NODE; 
    -        }
    -        else if (coresPerNode > (TSEsPerNode*2))
    -        {
    -             numESPsPerNode = TSEsPerNode;
    -        }
    -        else if (TSEsPerNode > 1)
    -        {
    -             numESPsPerNode = TSEsPerNode/2;
    -        }
    -        else // not really needed since numESPsPerNode is set to 1 from above
    -        {
    -             numESPsPerNode = DEFAULT_ESPS_PER_NODE;
    -        }
    -        
    -
    -        ftoa_((float)(numESPsPerNode)/(float)(coresPerNode), valuestr);
    +        float espsPerCore = computeNumESPsPerCore(FALSE);
    --- End diff --
    
    Per Hans' comment, should the parameter here be a test of the new CQD value?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: fix JIRA-1739 to aggressively al...

Posted by nonstop-qfchen <gi...@git.apache.org>.
Github user nonstop-qfchen commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/250#discussion_r49122144
  
    --- Diff: core/sql/sqlcomp/nadefaults.cpp ---
    @@ -4333,59 +4336,8 @@ void NADefaults::updateSystemParameters(NABoolean reInit)
     
         case MAX_ESPS_PER_CPU_PER_OP:
           {
    -        // set 2 ESPs per node, as a starting point.
    -        #define DEFAULT_ESPS_PER_NODE 2
    -
    -        Lng32 numESPsPerNode = DEFAULT_ESPS_PER_NODE;
    -        Lng32 coresPerNode = 1;        
    -          // Make sure the gpClusterInfo points at an NAClusterLinux object.
    -          // In osim simulation mode, the pointer can point at a NAClusterNSK
    -          // object, for which the method numTSEsForPOS() is not defined.
    -        NAClusterInfoLinux* gpLinux = dynamic_cast<NAClusterInfoLinux*>(gpClusterInfo);
    -
    -          // number of POS TSE
    -        Lng32 numTSEsPerCluster = gpLinux->numTSEsForPOS();
    -
    -          // cluster nodes
    -        Lng32 nodesdPerCluster = gpClusterInfo->getTotalNumberOfCPUs();
    -
    -          // TSEs per node
    -        Lng32 TSEsPerNode = numTSEsPerCluster/nodesdPerCluster;
    -
    -          // cores per node
    -        coresPerNode = gpClusterInfo->numberOfCpusPerSMP();
    -
    -          // For Linux/nt, we conservatively allocate ESPs per node as follows
    -          // - 1 ESP per 2 cpu cores if cores are equal or less than TSEs
    -          // - 1 ESP per TSE if number of cores is more than double the TSEs
    -          // - 1 ESP per 2 TSEs if cores are more than TSEs but less than double the TSEs
    -          // - 1 ESP per node. Only possible on NT or workstations
    -          //      - number of cores less than TSEs and there are 1 or 2 cpur cores per node
    -          //      - number of TSEs is less than cpu cores and there 1 or 2 TSEs per node.
    -          //        This case is probable if virtual nodes are used
    -
    -          // TSEsPerNode is 0 for arkcmps started by the seapilot universal comsumers
    -          // in this case we only consider cpu cores
    -        if ((coresPerNode <= TSEsPerNode) || (TSEsPerNode == 0))
    -        {
    -            if (coresPerNode > 1)
    -                numESPsPerNode = DEFAULT_ESPS_PER_NODE; 
    -        }
    -        else if (coresPerNode > (TSEsPerNode*2))
    -        {
    -             numESPsPerNode = TSEsPerNode;
    -        }
    -        else if (TSEsPerNode > 1)
    -        {
    -             numESPsPerNode = TSEsPerNode/2;
    -        }
    -        else // not really needed since numESPsPerNode is set to 1 from above
    -        {
    -             numESPsPerNode = DEFAULT_ESPS_PER_NODE;
    -        }
    -        
    -
    -        ftoa_((float)(numESPsPerNode)/(float)(coresPerNode), valuestr);
    +        float espsPerCore = computeNumESPsPerCore(FALSE);
    --- End diff --
    
    NADefaults::updateSystemParameters() is a bootstrap function, called when
    construct a new NADefaults object, or during the initialization of OSIM.
    
    During the call, a subset of the CQDs are influenced by the system
    parameters, such as # of nodes.
    
    So I think it makes sense to pass FALSE to it for now. Once we decide to
    turn on the aggressive mode, we need to change it to TRUE.
    
    Thanks
    
    On Thu, Jan 7, 2016 at 2:12 PM, Hans Zeller <no...@github.com>
    wrote:
    
    > In core/sql/sqlcomp/nadefaults.cpp
    > <https://github.com/apache/incubator-trafodion/pull/250#discussion_r49120076>
    > :
    >
    > > -        else if (coresPerNode > (TSEsPerNode*2))
    > > -        {
    > > -             numESPsPerNode = TSEsPerNode;
    > > -        }
    > > -        else if (TSEsPerNode > 1)
    > > -        {
    > > -             numESPsPerNode = TSEsPerNode/2;
    > > -        }
    > > -        else // not really needed since numESPsPerNode is set to 1 from above
    > > -        {
    > > -             numESPsPerNode = DEFAULT_ESPS_PER_NODE;
    > > -        }
    > > -
    > > -
    > > -        ftoa_((float)(numESPsPerNode)/(float)(coresPerNode), valuestr);
    > > +        float espsPerCore = computeNumESPsPerCore(FALSE);
    >
    > It makes a LOT more sense now with the complete code. Wouldn't it be more
    > consistent if we also would use the AGGRESSIVE_ESP_ALLOCATION_PER_CORE CQD
    > here instead of the hard-coded FALSE? Is this method called before or after
    > reading the system defaults table?
    >
    > —
    > Reply to this email directly or view it on GitHub
    > <https://github.com/apache/incubator-trafodion/pull/250/files#r49120076>.
    >
    
    
    
    -- 
    Regards, --Qifan



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: fix JIRA-1739 to aggressively al...

Posted by nonstop-qfchen <gi...@git.apache.org>.
Github user nonstop-qfchen commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/250#discussion_r49027797
  
    --- Diff: core/sql/sqlcomp/nadefaults.cpp ---
    @@ -5914,6 +5866,73 @@ enum DefaultConstants NADefaults::validateAndInsert(const char *attrName,
     
     } // NADefaults::validateAndInsert()
     
    +float NADefaults::computeNumESPsPerCore(NABoolean aggressive)
    +{
    +   #define DEFAULT_ESPS_PER_NODE 2   // for conservation allocation
    +   #define DEFAULT_ESPS_PER_CORE 0.5 // for aggressive allocation
    +
    +     // Make sure the gpClusterInfo points at an NAClusterLinux object.
    +     // In osim simulation mode, the pointer can point at a NAClusterNSK
    +     // object, for which the method numTSEsForPOS() is not defined.
    +   NAClusterInfoLinux* gpLinux = dynamic_cast<NAClusterInfoLinux*>(gpClusterInfo);
    +   assert(gpLinux);		
    +
    +   // cores per node
    +   Lng32 coresPerNode = gpClusterInfo->numberOfCpusPerSMP();
    +
    +   if ( aggressive ) {
    +      float totalMemory = gpLinux->totalMemoryAvailable(); // per Node, in KB
    +      totalMemory /= 1024; // per Node, in GB
    +      totalMemory /= coresPerNode ; // per core, in GB
    +      totalMemory /= 2; // per core, 2GB per ESP
    +      return MINOF(DEFAULT_ESPS_PER_CORE, totalMemory);
    +   }
    +
    +   Lng32 numESPsPerNode = DEFAULT_ESPS_PER_NODE;
    +
    +     // number of POS TSE
    +   Lng32 numTSEsPerCluster = gpLinux->numTSEsForPOS();
    +
    +     // cluster nodes
    +   Lng32 nodesdPerCluster = gpClusterInfo->getTotalNumberOfCPUs();
    +
    +     // TSEs per node
    +   Lng32 TSEsPerNode = numTSEsPerCluster/nodesdPerCluster;
    +
    +
    +
    +     // For Linux/nt, we conservatively allocate ESPs per node as follows
    +     // - 1 ESP per 2 cpu cores if cores are equal or less than TSEs
    +     // - 1 ESP per TSE if number of cores is more than double the TSEs
    +     // - 1 ESP per 2 TSEs if cores are more than TSEs but less than double the TSEs
    +     // - 1 ESP per node. Only possible on NT or workstations
    +     //      - number of cores less than TSEs and there are 1 or 2 cpur cores per node
    +     //      - number of TSEs is less than cpu cores and there 1 or 2 TSEs per node.
    +     //        This case is probable if virtual nodes are used
    +
    +     // TSEsPerNode is 0 for arkcmps started by the seapilot universal comsumers
    +     // in this case we only consider cpu cores
    +   if ( coresPerNode <= TSEsPerNode || TSEsPerNode == 0 )
    +   {
    +       if (coresPerNode > 1)
    +           numESPsPerNode = DEFAULT_ESPS_PER_NODE; 
    +   }
    +   else if (coresPerNode > (TSEsPerNode*2))
    +   {
    +        numESPsPerNode = TSEsPerNode;
    +   }
    +   else if (TSEsPerNode > 1)
    +   {
    +        numESPsPerNode = TSEsPerNode/2;
    +   }
    +   else // not really needed since numESPsPerNode is set to 1 from above
    +   {
    +        numESPsPerNode = DEFAULT_ESPS_PER_NODE;
    +   }
    +        
    +   return (float)(numESPsPerNode)/(float)(coresPerNode);
    --- End diff --
    
    yes. the code is inherited from the work in HP and computes # of ESps very conservatively. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: fix JIRA-1739 to aggressively al...

Posted by nonstop-qfchen <gi...@git.apache.org>.
Github user nonstop-qfchen commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/250#discussion_r49026922
  
    --- Diff: core/sql/sqlcomp/nadefaults.cpp ---
    @@ -5914,6 +5866,73 @@ enum DefaultConstants NADefaults::validateAndInsert(const char *attrName,
     
     } // NADefaults::validateAndInsert()
     
    +float NADefaults::computeNumESPsPerCore(NABoolean aggressive)
    +{
    +   #define DEFAULT_ESPS_PER_NODE 2   // for conservation allocation
    +   #define DEFAULT_ESPS_PER_CORE 0.5 // for aggressive allocation
    +
    +     // Make sure the gpClusterInfo points at an NAClusterLinux object.
    +     // In osim simulation mode, the pointer can point at a NAClusterNSK
    +     // object, for which the method numTSEsForPOS() is not defined.
    +   NAClusterInfoLinux* gpLinux = dynamic_cast<NAClusterInfoLinux*>(gpClusterInfo);
    +   assert(gpLinux);		
    +
    +   // cores per node
    +   Lng32 coresPerNode = gpClusterInfo->numberOfCpusPerSMP();
    +
    +   if ( aggressive ) {
    +      float totalMemory = gpLinux->totalMemoryAvailable(); // per Node, in KB
    +      totalMemory /= 1024; // per Node, in GB
    --- End diff --
    
    yes, it is turned off now.  The performance team is running a test with the
    feature turned on. Once everything is OK, the feature will be turned on by
    default.
    
    On Wed, Jan 6, 2016 at 5:17 PM, Hans Zeller <no...@github.com>
    wrote:
    
    > In core/sql/sqlcomp/nadefaults.cpp
    > <https://github.com/apache/incubator-trafodion/pull/250#discussion_r49023626>
    > :
    >
    > > +{
    > > +   #define DEFAULT_ESPS_PER_NODE 2   // for conservation allocation
    > > +   #define DEFAULT_ESPS_PER_CORE 0.5 // for aggressive allocation
    > > +
    > > +     // Make sure the gpClusterInfo points at an NAClusterLinux object.
    > > +     // In osim simulation mode, the pointer can point at a NAClusterNSK
    > > +     // object, for which the method numTSEsForPOS() is not defined.
    > > +   NAClusterInfoLinux* gpLinux = dynamic_cast<NAClusterInfoLinux*>(gpClusterInfo);
    > > +   assert(gpLinux);		
    > > +
    > > +   // cores per node
    > > +   Lng32 coresPerNode = gpClusterInfo->numberOfCpusPerSMP();
    > > +
    > > +   if ( aggressive ) {
    > > +      float totalMemory = gpLinux->totalMemoryAvailable(); // per Node, in KB
    > > +      totalMemory /= 1024; // per Node, in GB
    >
    > For conversion KB to GB we would need to divide by 1024*1024?
    >
    > —
    > Reply to this email directly or view it on GitHub
    > <https://github.com/apache/incubator-trafodion/pull/250/files#r49023626>.
    >
    
    
    
    -- 
    Regards, --Qifan



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: fix JIRA-1739 to aggressively al...

Posted by nonstop-qfchen <gi...@git.apache.org>.
Github user nonstop-qfchen commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/250#discussion_r49027110
  
    --- Diff: core/sql/sqlcomp/nadefaults.cpp ---
    @@ -5914,6 +5866,73 @@ enum DefaultConstants NADefaults::validateAndInsert(const char *attrName,
     
     } // NADefaults::validateAndInsert()
     
    +float NADefaults::computeNumESPsPerCore(NABoolean aggressive)
    +{
    +   #define DEFAULT_ESPS_PER_NODE 2   // for conservation allocation
    +   #define DEFAULT_ESPS_PER_CORE 0.5 // for aggressive allocation
    +
    +     // Make sure the gpClusterInfo points at an NAClusterLinux object.
    +     // In osim simulation mode, the pointer can point at a NAClusterNSK
    +     // object, for which the method numTSEsForPOS() is not defined.
    +   NAClusterInfoLinux* gpLinux = dynamic_cast<NAClusterInfoLinux*>(gpClusterInfo);
    +   assert(gpLinux);		
    +
    +   // cores per node
    +   Lng32 coresPerNode = gpClusterInfo->numberOfCpusPerSMP();
    +
    +   if ( aggressive ) {
    +      float totalMemory = gpLinux->totalMemoryAvailable(); // per Node, in KB
    +      totalMemory /= 1024; // per Node, in GB
    +      totalMemory /= coresPerNode ; // per core, in GB
    +      totalMemory /= 2; // per core, 2GB per ESP
    --- End diff --
    
    Maybe wait until we receive some data on the default setting?
    
    On Wed, Jan 6, 2016 at 5:18 PM, Hans Zeller <no...@github.com>
    wrote:
    
    > In core/sql/sqlcomp/nadefaults.cpp
    > <https://github.com/apache/incubator-trafodion/pull/250#discussion_r49023709>
    > :
    >
    > > +   #define DEFAULT_ESPS_PER_CORE 0.5 // for aggressive allocation
    > > +
    > > +     // Make sure the gpClusterInfo points at an NAClusterLinux object.
    > > +     // In osim simulation mode, the pointer can point at a NAClusterNSK
    > > +     // object, for which the method numTSEsForPOS() is not defined.
    > > +   NAClusterInfoLinux* gpLinux = dynamic_cast<NAClusterInfoLinux*>(gpClusterInfo);
    > > +   assert(gpLinux);		
    > > +
    > > +   // cores per node
    > > +   Lng32 coresPerNode = gpClusterInfo->numberOfCpusPerSMP();
    > > +
    > > +   if ( aggressive ) {
    > > +      float totalMemory = gpLinux->totalMemoryAvailable(); // per Node, in KB
    > > +      totalMemory /= 1024; // per Node, in GB
    > > +      totalMemory /= coresPerNode ; // per core, in GB
    > > +      totalMemory /= 2; // per core, 2GB per ESP
    >
    > Should this be another CQD?
    >
    > —
    > Reply to this email directly or view it on GitHub
    > <https://github.com/apache/incubator-trafodion/pull/250/files#r49023709>.
    >
    
    
    
    -- 
    Regards, --Qifan



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: fix JIRA-1739 to aggressively al...

Posted by nonstop-qfchen <gi...@git.apache.org>.
Github user nonstop-qfchen commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/250#discussion_r49027875
  
    --- Diff: core/sql/sqlcomp/nadefaults.cpp ---
    @@ -5914,6 +5866,73 @@ enum DefaultConstants NADefaults::validateAndInsert(const char *attrName,
     
     } // NADefaults::validateAndInsert()
     
    +float NADefaults::computeNumESPsPerCore(NABoolean aggressive)
    +{
    +   #define DEFAULT_ESPS_PER_NODE 2   // for conservation allocation
    +   #define DEFAULT_ESPS_PER_CORE 0.5 // for aggressive allocation
    +
    +     // Make sure the gpClusterInfo points at an NAClusterLinux object.
    +     // In osim simulation mode, the pointer can point at a NAClusterNSK
    +     // object, for which the method numTSEsForPOS() is not defined.
    +   NAClusterInfoLinux* gpLinux = dynamic_cast<NAClusterInfoLinux*>(gpClusterInfo);
    +   assert(gpLinux);		
    +
    +   // cores per node
    +   Lng32 coresPerNode = gpClusterInfo->numberOfCpusPerSMP();
    +
    +   if ( aggressive ) {
    +      float totalMemory = gpLinux->totalMemoryAvailable(); // per Node, in KB
    --- End diff --
    
    yes, that is another to do it. But the difference is that the fix takes into consideration both #cores and memory per node. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: fix JIRA-1739 to aggressively al...

Posted by nonstop-qfchen <gi...@git.apache.org>.
Github user nonstop-qfchen commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/250#discussion_r49027690
  
    --- Diff: core/sql/sqlcomp/nadefaults.cpp ---
    @@ -5914,6 +5866,73 @@ enum DefaultConstants NADefaults::validateAndInsert(const char *attrName,
     
     } // NADefaults::validateAndInsert()
     
    +float NADefaults::computeNumESPsPerCore(NABoolean aggressive)
    +{
    +   #define DEFAULT_ESPS_PER_NODE 2   // for conservation allocation
    +   #define DEFAULT_ESPS_PER_CORE 0.5 // for aggressive allocation
    +
    +     // Make sure the gpClusterInfo points at an NAClusterLinux object.
    +     // In osim simulation mode, the pointer can point at a NAClusterNSK
    +     // object, for which the method numTSEsForPOS() is not defined.
    +   NAClusterInfoLinux* gpLinux = dynamic_cast<NAClusterInfoLinux*>(gpClusterInfo);
    +   assert(gpLinux);		
    +
    +   // cores per node
    +   Lng32 coresPerNode = gpClusterInfo->numberOfCpusPerSMP();
    +
    +   if ( aggressive ) {
    +      float totalMemory = gpLinux->totalMemoryAvailable(); // per Node, in KB
    +      totalMemory /= 1024; // per Node, in GB
    +      totalMemory /= coresPerNode ; // per core, in GB
    +      totalMemory /= 2; // per core, 2GB per ESP
    --- End diff --
    
    I have submitted a fix to the KB->GB conversion typo.
    
    On Wed, Jan 6, 2016 at 6:01 PM, Qifan Chen <qi...@esgyn.com> wrote:
    
    > Maybe wait until we receive some data on the default setting?
    >
    > On Wed, Jan 6, 2016 at 5:18 PM, Hans Zeller <no...@github.com>
    > wrote:
    >
    >> In core/sql/sqlcomp/nadefaults.cpp
    >> <https://github.com/apache/incubator-trafodion/pull/250#discussion_r49023709>
    >> :
    >>
    >> > +   #define DEFAULT_ESPS_PER_CORE 0.5 // for aggressive allocation
    >> > +
    >> > +     // Make sure the gpClusterInfo points at an NAClusterLinux object.
    >> > +     // In osim simulation mode, the pointer can point at a NAClusterNSK
    >> > +     // object, for which the method numTSEsForPOS() is not defined.
    >> > +   NAClusterInfoLinux* gpLinux = dynamic_cast<NAClusterInfoLinux*>(gpClusterInfo);
    >> > +   assert(gpLinux);		
    >> > +
    >> > +   // cores per node
    >> > +   Lng32 coresPerNode = gpClusterInfo->numberOfCpusPerSMP();
    >> > +
    >> > +   if ( aggressive ) {
    >> > +      float totalMemory = gpLinux->totalMemoryAvailable(); // per Node, in KB
    >> > +      totalMemory /= 1024; // per Node, in GB
    >> > +      totalMemory /= coresPerNode ; // per core, in GB
    >> > +      totalMemory /= 2; // per core, 2GB per ESP
    >>
    >> Should this be another CQD?
    >>
    >> —
    >> Reply to this email directly or view it on GitHub
    >> <https://github.com/apache/incubator-trafodion/pull/250/files#r49023709>.
    >>
    >
    >
    >
    > --
    > Regards, --Qifan
    >
    >
    
    
    -- 
    Regards, --Qifan



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: fix JIRA-1739 to aggressively al...

Posted by zellerh <gi...@git.apache.org>.
Github user zellerh commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/250#discussion_r49023989
  
    --- Diff: core/sql/sqlcomp/nadefaults.cpp ---
    @@ -5914,6 +5866,73 @@ enum DefaultConstants NADefaults::validateAndInsert(const char *attrName,
     
     } // NADefaults::validateAndInsert()
     
    +float NADefaults::computeNumESPsPerCore(NABoolean aggressive)
    +{
    +   #define DEFAULT_ESPS_PER_NODE 2   // for conservation allocation
    +   #define DEFAULT_ESPS_PER_CORE 0.5 // for aggressive allocation
    +
    +     // Make sure the gpClusterInfo points at an NAClusterLinux object.
    +     // In osim simulation mode, the pointer can point at a NAClusterNSK
    +     // object, for which the method numTSEsForPOS() is not defined.
    +   NAClusterInfoLinux* gpLinux = dynamic_cast<NAClusterInfoLinux*>(gpClusterInfo);
    +   assert(gpLinux);		
    +
    +   // cores per node
    +   Lng32 coresPerNode = gpClusterInfo->numberOfCpusPerSMP();
    +
    +   if ( aggressive ) {
    +      float totalMemory = gpLinux->totalMemoryAvailable(); // per Node, in KB
    +      totalMemory /= 1024; // per Node, in GB
    +      totalMemory /= coresPerNode ; // per core, in GB
    +      totalMemory /= 2; // per core, 2GB per ESP
    +      return MINOF(DEFAULT_ESPS_PER_CORE, totalMemory);
    +   }
    +
    +   Lng32 numESPsPerNode = DEFAULT_ESPS_PER_NODE;
    +
    +     // number of POS TSE
    +   Lng32 numTSEsPerCluster = gpLinux->numTSEsForPOS();
    --- End diff --
    
    Since there are no TSEs in Trafodion, can we get rid of this code?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: fix JIRA-1739 to aggressively al...

Posted by nonstop-qfchen <gi...@git.apache.org>.
Github user nonstop-qfchen commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/250#discussion_r49094833
  
    --- Diff: core/sql/sqlcomp/nadefaults.cpp ---
    @@ -4333,59 +4336,8 @@ void NADefaults::updateSystemParameters(NABoolean reInit)
     
         case MAX_ESPS_PER_CPU_PER_OP:
           {
    -        // set 2 ESPs per node, as a starting point.
    -        #define DEFAULT_ESPS_PER_NODE 2
    -
    -        Lng32 numESPsPerNode = DEFAULT_ESPS_PER_NODE;
    -        Lng32 coresPerNode = 1;        
    -          // Make sure the gpClusterInfo points at an NAClusterLinux object.
    -          // In osim simulation mode, the pointer can point at a NAClusterNSK
    -          // object, for which the method numTSEsForPOS() is not defined.
    -        NAClusterInfoLinux* gpLinux = dynamic_cast<NAClusterInfoLinux*>(gpClusterInfo);
    -
    -          // number of POS TSE
    -        Lng32 numTSEsPerCluster = gpLinux->numTSEsForPOS();
    -
    -          // cluster nodes
    -        Lng32 nodesdPerCluster = gpClusterInfo->getTotalNumberOfCPUs();
    -
    -          // TSEs per node
    -        Lng32 TSEsPerNode = numTSEsPerCluster/nodesdPerCluster;
    -
    -          // cores per node
    -        coresPerNode = gpClusterInfo->numberOfCpusPerSMP();
    -
    -          // For Linux/nt, we conservatively allocate ESPs per node as follows
    -          // - 1 ESP per 2 cpu cores if cores are equal or less than TSEs
    -          // - 1 ESP per TSE if number of cores is more than double the TSEs
    -          // - 1 ESP per 2 TSEs if cores are more than TSEs but less than double the TSEs
    -          // - 1 ESP per node. Only possible on NT or workstations
    -          //      - number of cores less than TSEs and there are 1 or 2 cpur cores per node
    -          //      - number of TSEs is less than cpu cores and there 1 or 2 TSEs per node.
    -          //        This case is probable if virtual nodes are used
    -
    -          // TSEsPerNode is 0 for arkcmps started by the seapilot universal comsumers
    -          // in this case we only consider cpu cores
    -        if ((coresPerNode <= TSEsPerNode) || (TSEsPerNode == 0))
    -        {
    -            if (coresPerNode > 1)
    -                numESPsPerNode = DEFAULT_ESPS_PER_NODE; 
    -        }
    -        else if (coresPerNode > (TSEsPerNode*2))
    -        {
    -             numESPsPerNode = TSEsPerNode;
    -        }
    -        else if (TSEsPerNode > 1)
    -        {
    -             numESPsPerNode = TSEsPerNode/2;
    -        }
    -        else // not really needed since numESPsPerNode is set to 1 from above
    -        {
    -             numESPsPerNode = DEFAULT_ESPS_PER_NODE;
    -        }
    -        
    -
    -        ftoa_((float)(numESPsPerNode)/(float)(coresPerNode), valuestr);
    +        float espsPerCore = computeNumESPsPerCore(FALSE);
    --- End diff --
    
    The performance test will test the aggressive mode first, without additional parameters to adjust. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: fix JIRA-1739 to aggressively al...

Posted by zellerh <gi...@git.apache.org>.
Github user zellerh commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/250#discussion_r49120076
  
    --- Diff: core/sql/sqlcomp/nadefaults.cpp ---
    @@ -4333,59 +4336,8 @@ void NADefaults::updateSystemParameters(NABoolean reInit)
     
         case MAX_ESPS_PER_CPU_PER_OP:
           {
    -        // set 2 ESPs per node, as a starting point.
    -        #define DEFAULT_ESPS_PER_NODE 2
    -
    -        Lng32 numESPsPerNode = DEFAULT_ESPS_PER_NODE;
    -        Lng32 coresPerNode = 1;        
    -          // Make sure the gpClusterInfo points at an NAClusterLinux object.
    -          // In osim simulation mode, the pointer can point at a NAClusterNSK
    -          // object, for which the method numTSEsForPOS() is not defined.
    -        NAClusterInfoLinux* gpLinux = dynamic_cast<NAClusterInfoLinux*>(gpClusterInfo);
    -
    -          // number of POS TSE
    -        Lng32 numTSEsPerCluster = gpLinux->numTSEsForPOS();
    -
    -          // cluster nodes
    -        Lng32 nodesdPerCluster = gpClusterInfo->getTotalNumberOfCPUs();
    -
    -          // TSEs per node
    -        Lng32 TSEsPerNode = numTSEsPerCluster/nodesdPerCluster;
    -
    -          // cores per node
    -        coresPerNode = gpClusterInfo->numberOfCpusPerSMP();
    -
    -          // For Linux/nt, we conservatively allocate ESPs per node as follows
    -          // - 1 ESP per 2 cpu cores if cores are equal or less than TSEs
    -          // - 1 ESP per TSE if number of cores is more than double the TSEs
    -          // - 1 ESP per 2 TSEs if cores are more than TSEs but less than double the TSEs
    -          // - 1 ESP per node. Only possible on NT or workstations
    -          //      - number of cores less than TSEs and there are 1 or 2 cpur cores per node
    -          //      - number of TSEs is less than cpu cores and there 1 or 2 TSEs per node.
    -          //        This case is probable if virtual nodes are used
    -
    -          // TSEsPerNode is 0 for arkcmps started by the seapilot universal comsumers
    -          // in this case we only consider cpu cores
    -        if ((coresPerNode <= TSEsPerNode) || (TSEsPerNode == 0))
    -        {
    -            if (coresPerNode > 1)
    -                numESPsPerNode = DEFAULT_ESPS_PER_NODE; 
    -        }
    -        else if (coresPerNode > (TSEsPerNode*2))
    -        {
    -             numESPsPerNode = TSEsPerNode;
    -        }
    -        else if (TSEsPerNode > 1)
    -        {
    -             numESPsPerNode = TSEsPerNode/2;
    -        }
    -        else // not really needed since numESPsPerNode is set to 1 from above
    -        {
    -             numESPsPerNode = DEFAULT_ESPS_PER_NODE;
    -        }
    -        
    -
    -        ftoa_((float)(numESPsPerNode)/(float)(coresPerNode), valuestr);
    +        float espsPerCore = computeNumESPsPerCore(FALSE);
    --- End diff --
    
    It makes a LOT more sense now with the complete code. Wouldn't it be more consistent if we also would use the AGGRESSIVE_ESP_ALLOCATION_PER_CORE CQD here instead of the hard-coded FALSE? Is this method called before or after reading the system defaults table?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: fix JIRA-1739 to aggressively al...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-trafodion/pull/250


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: fix JIRA-1739 to aggressively al...

Posted by nonstop-qfchen <gi...@git.apache.org>.
Github user nonstop-qfchen commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/250#discussion_r49027967
  
    --- Diff: core/sql/sqlcomp/nadefaults.cpp ---
    @@ -5914,6 +5866,73 @@ enum DefaultConstants NADefaults::validateAndInsert(const char *attrName,
     
     } // NADefaults::validateAndInsert()
     
    +float NADefaults::computeNumESPsPerCore(NABoolean aggressive)
    +{
    +   #define DEFAULT_ESPS_PER_NODE 2   // for conservation allocation
    +   #define DEFAULT_ESPS_PER_CORE 0.5 // for aggressive allocation
    +
    +     // Make sure the gpClusterInfo points at an NAClusterLinux object.
    +     // In osim simulation mode, the pointer can point at a NAClusterNSK
    +     // object, for which the method numTSEsForPOS() is not defined.
    +   NAClusterInfoLinux* gpLinux = dynamic_cast<NAClusterInfoLinux*>(gpClusterInfo);
    +   assert(gpLinux);		
    +
    +   // cores per node
    +   Lng32 coresPerNode = gpClusterInfo->numberOfCpusPerSMP();
    +
    +   if ( aggressive ) {
    +      float totalMemory = gpLinux->totalMemoryAvailable(); // per Node, in KB
    --- End diff --
    
    once the performance of the change is done, we will permanently turn on the feature. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: fix JIRA-1739 to aggressively al...

Posted by zellerh <gi...@git.apache.org>.
Github user zellerh commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/250#discussion_r49024086
  
    --- Diff: core/sql/sqlcomp/nadefaults.cpp ---
    @@ -5914,6 +5866,73 @@ enum DefaultConstants NADefaults::validateAndInsert(const char *attrName,
     
     } // NADefaults::validateAndInsert()
     
    +float NADefaults::computeNumESPsPerCore(NABoolean aggressive)
    +{
    +   #define DEFAULT_ESPS_PER_NODE 2   // for conservation allocation
    +   #define DEFAULT_ESPS_PER_CORE 0.5 // for aggressive allocation
    +
    +     // Make sure the gpClusterInfo points at an NAClusterLinux object.
    +     // In osim simulation mode, the pointer can point at a NAClusterNSK
    +     // object, for which the method numTSEsForPOS() is not defined.
    +   NAClusterInfoLinux* gpLinux = dynamic_cast<NAClusterInfoLinux*>(gpClusterInfo);
    +   assert(gpLinux);		
    +
    +   // cores per node
    +   Lng32 coresPerNode = gpClusterInfo->numberOfCpusPerSMP();
    +
    +   if ( aggressive ) {
    +      float totalMemory = gpLinux->totalMemoryAvailable(); // per Node, in KB
    +      totalMemory /= 1024; // per Node, in GB
    +      totalMemory /= coresPerNode ; // per core, in GB
    +      totalMemory /= 2; // per core, 2GB per ESP
    +      return MINOF(DEFAULT_ESPS_PER_CORE, totalMemory);
    +   }
    +
    +   Lng32 numESPsPerNode = DEFAULT_ESPS_PER_NODE;
    +
    +     // number of POS TSE
    +   Lng32 numTSEsPerCluster = gpLinux->numTSEsForPOS();
    +
    +     // cluster nodes
    +   Lng32 nodesdPerCluster = gpClusterInfo->getTotalNumberOfCPUs();
    +
    +     // TSEs per node
    +   Lng32 TSEsPerNode = numTSEsPerCluster/nodesdPerCluster;
    +
    +
    +
    +     // For Linux/nt, we conservatively allocate ESPs per node as follows
    +     // - 1 ESP per 2 cpu cores if cores are equal or less than TSEs
    +     // - 1 ESP per TSE if number of cores is more than double the TSEs
    +     // - 1 ESP per 2 TSEs if cores are more than TSEs but less than double the TSEs
    +     // - 1 ESP per node. Only possible on NT or workstations
    +     //      - number of cores less than TSEs and there are 1 or 2 cpur cores per node
    +     //      - number of TSEs is less than cpu cores and there 1 or 2 TSEs per node.
    +     //        This case is probable if virtual nodes are used
    +
    +     // TSEsPerNode is 0 for arkcmps started by the seapilot universal comsumers
    +     // in this case we only consider cpu cores
    +   if ( coresPerNode <= TSEsPerNode || TSEsPerNode == 0 )
    +   {
    +       if (coresPerNode > 1)
    +           numESPsPerNode = DEFAULT_ESPS_PER_NODE; 
    --- End diff --
    
    It's already mentioned below in a comment on line 5928, this line also does nothing.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: fix JIRA-1739 to aggressively al...

Posted by nonstop-qfchen <gi...@git.apache.org>.
Github user nonstop-qfchen commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/250#discussion_r49026982
  
    --- Diff: core/sql/sqlcomp/nadefaults.cpp ---
    @@ -5914,6 +5866,73 @@ enum DefaultConstants NADefaults::validateAndInsert(const char *attrName,
     
     } // NADefaults::validateAndInsert()
     
    +float NADefaults::computeNumESPsPerCore(NABoolean aggressive)
    +{
    +   #define DEFAULT_ESPS_PER_NODE 2   // for conservation allocation
    +   #define DEFAULT_ESPS_PER_CORE 0.5 // for aggressive allocation
    +
    +     // Make sure the gpClusterInfo points at an NAClusterLinux object.
    +     // In osim simulation mode, the pointer can point at a NAClusterNSK
    +     // object, for which the method numTSEsForPOS() is not defined.
    +   NAClusterInfoLinux* gpLinux = dynamic_cast<NAClusterInfoLinux*>(gpClusterInfo);
    +   assert(gpLinux);		
    +
    +   // cores per node
    +   Lng32 coresPerNode = gpClusterInfo->numberOfCpusPerSMP();
    +
    +   if ( aggressive ) {
    +      float totalMemory = gpLinux->totalMemoryAvailable(); // per Node, in KB
    +      totalMemory /= 1024; // per Node, in GB
    --- End diff --
    
    good point.  I'll add the fix.
    
    On Wed, Jan 6, 2016 at 5:17 PM, Hans Zeller <no...@github.com>
    wrote:
    
    > In core/sql/sqlcomp/nadefaults.cpp
    > <https://github.com/apache/incubator-trafodion/pull/250#discussion_r49023626>
    > :
    >
    > > +{
    > > +   #define DEFAULT_ESPS_PER_NODE 2   // for conservation allocation
    > > +   #define DEFAULT_ESPS_PER_CORE 0.5 // for aggressive allocation
    > > +
    > > +     // Make sure the gpClusterInfo points at an NAClusterLinux object.
    > > +     // In osim simulation mode, the pointer can point at a NAClusterNSK
    > > +     // object, for which the method numTSEsForPOS() is not defined.
    > > +   NAClusterInfoLinux* gpLinux = dynamic_cast<NAClusterInfoLinux*>(gpClusterInfo);
    > > +   assert(gpLinux);		
    > > +
    > > +   // cores per node
    > > +   Lng32 coresPerNode = gpClusterInfo->numberOfCpusPerSMP();
    > > +
    > > +   if ( aggressive ) {
    > > +      float totalMemory = gpLinux->totalMemoryAvailable(); // per Node, in KB
    > > +      totalMemory /= 1024; // per Node, in GB
    >
    > For conversion KB to GB we would need to divide by 1024*1024?
    >
    > —
    > Reply to this email directly or view it on GitHub
    > <https://github.com/apache/incubator-trafodion/pull/250/files#r49023626>.
    >
    
    
    
    -- 
    Regards, --Qifan



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: fix JIRA-1739 to aggressively al...

Posted by zellerh <gi...@git.apache.org>.
Github user zellerh commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/250#discussion_r49023626
  
    --- Diff: core/sql/sqlcomp/nadefaults.cpp ---
    @@ -5914,6 +5866,73 @@ enum DefaultConstants NADefaults::validateAndInsert(const char *attrName,
     
     } // NADefaults::validateAndInsert()
     
    +float NADefaults::computeNumESPsPerCore(NABoolean aggressive)
    +{
    +   #define DEFAULT_ESPS_PER_NODE 2   // for conservation allocation
    +   #define DEFAULT_ESPS_PER_CORE 0.5 // for aggressive allocation
    +
    +     // Make sure the gpClusterInfo points at an NAClusterLinux object.
    +     // In osim simulation mode, the pointer can point at a NAClusterNSK
    +     // object, for which the method numTSEsForPOS() is not defined.
    +   NAClusterInfoLinux* gpLinux = dynamic_cast<NAClusterInfoLinux*>(gpClusterInfo);
    +   assert(gpLinux);		
    +
    +   // cores per node
    +   Lng32 coresPerNode = gpClusterInfo->numberOfCpusPerSMP();
    +
    +   if ( aggressive ) {
    +      float totalMemory = gpLinux->totalMemoryAvailable(); // per Node, in KB
    +      totalMemory /= 1024; // per Node, in GB
    --- End diff --
    
    For conversion KB to GB we would need to divide by 1024*1024?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: fix JIRA-1739 to aggressively al...

Posted by zellerh <gi...@git.apache.org>.
Github user zellerh commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/250#discussion_r49126403
  
    --- Diff: core/sql/sqlcomp/nadefaults.cpp ---
    @@ -4333,59 +4336,8 @@ void NADefaults::updateSystemParameters(NABoolean reInit)
     
         case MAX_ESPS_PER_CPU_PER_OP:
           {
    -        // set 2 ESPs per node, as a starting point.
    -        #define DEFAULT_ESPS_PER_NODE 2
    -
    -        Lng32 numESPsPerNode = DEFAULT_ESPS_PER_NODE;
    -        Lng32 coresPerNode = 1;        
    -          // Make sure the gpClusterInfo points at an NAClusterLinux object.
    -          // In osim simulation mode, the pointer can point at a NAClusterNSK
    -          // object, for which the method numTSEsForPOS() is not defined.
    -        NAClusterInfoLinux* gpLinux = dynamic_cast<NAClusterInfoLinux*>(gpClusterInfo);
    -
    -          // number of POS TSE
    -        Lng32 numTSEsPerCluster = gpLinux->numTSEsForPOS();
    -
    -          // cluster nodes
    -        Lng32 nodesdPerCluster = gpClusterInfo->getTotalNumberOfCPUs();
    -
    -          // TSEs per node
    -        Lng32 TSEsPerNode = numTSEsPerCluster/nodesdPerCluster;
    -
    -          // cores per node
    -        coresPerNode = gpClusterInfo->numberOfCpusPerSMP();
    -
    -          // For Linux/nt, we conservatively allocate ESPs per node as follows
    -          // - 1 ESP per 2 cpu cores if cores are equal or less than TSEs
    -          // - 1 ESP per TSE if number of cores is more than double the TSEs
    -          // - 1 ESP per 2 TSEs if cores are more than TSEs but less than double the TSEs
    -          // - 1 ESP per node. Only possible on NT or workstations
    -          //      - number of cores less than TSEs and there are 1 or 2 cpur cores per node
    -          //      - number of TSEs is less than cpu cores and there 1 or 2 TSEs per node.
    -          //        This case is probable if virtual nodes are used
    -
    -          // TSEsPerNode is 0 for arkcmps started by the seapilot universal comsumers
    -          // in this case we only consider cpu cores
    -        if ((coresPerNode <= TSEsPerNode) || (TSEsPerNode == 0))
    -        {
    -            if (coresPerNode > 1)
    -                numESPsPerNode = DEFAULT_ESPS_PER_NODE; 
    -        }
    -        else if (coresPerNode > (TSEsPerNode*2))
    -        {
    -             numESPsPerNode = TSEsPerNode;
    -        }
    -        else if (TSEsPerNode > 1)
    -        {
    -             numESPsPerNode = TSEsPerNode/2;
    -        }
    -        else // not really needed since numESPsPerNode is set to 1 from above
    -        {
    -             numESPsPerNode = DEFAULT_ESPS_PER_NODE;
    -        }
    -        
    -
    -        ftoa_((float)(numESPsPerNode)/(float)(coresPerNode), valuestr);
    +        float espsPerCore = computeNumESPsPerCore(FALSE);
    --- End diff --
    
    But, how could we do this if the aggressive mode is a CQD?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: fix JIRA-1739 to aggressively al...

Posted by nonstop-qfchen <gi...@git.apache.org>.
Github user nonstop-qfchen commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/250#discussion_r49111457
  
    --- Diff: core/sql/sqlcomp/nadefaults.cpp ---
    @@ -4333,59 +4336,8 @@ void NADefaults::updateSystemParameters(NABoolean reInit)
     
         case MAX_ESPS_PER_CPU_PER_OP:
           {
    -        // set 2 ESPs per node, as a starting point.
    -        #define DEFAULT_ESPS_PER_NODE 2
    -
    -        Lng32 numESPsPerNode = DEFAULT_ESPS_PER_NODE;
    -        Lng32 coresPerNode = 1;        
    -          // Make sure the gpClusterInfo points at an NAClusterLinux object.
    -          // In osim simulation mode, the pointer can point at a NAClusterNSK
    -          // object, for which the method numTSEsForPOS() is not defined.
    -        NAClusterInfoLinux* gpLinux = dynamic_cast<NAClusterInfoLinux*>(gpClusterInfo);
    -
    -          // number of POS TSE
    -        Lng32 numTSEsPerCluster = gpLinux->numTSEsForPOS();
    -
    -          // cluster nodes
    -        Lng32 nodesdPerCluster = gpClusterInfo->getTotalNumberOfCPUs();
    -
    -          // TSEs per node
    -        Lng32 TSEsPerNode = numTSEsPerCluster/nodesdPerCluster;
    -
    -          // cores per node
    -        coresPerNode = gpClusterInfo->numberOfCpusPerSMP();
    -
    -          // For Linux/nt, we conservatively allocate ESPs per node as follows
    -          // - 1 ESP per 2 cpu cores if cores are equal or less than TSEs
    -          // - 1 ESP per TSE if number of cores is more than double the TSEs
    -          // - 1 ESP per 2 TSEs if cores are more than TSEs but less than double the TSEs
    -          // - 1 ESP per node. Only possible on NT or workstations
    -          //      - number of cores less than TSEs and there are 1 or 2 cpur cores per node
    -          //      - number of TSEs is less than cpu cores and there 1 or 2 TSEs per node.
    -          //        This case is probable if virtual nodes are used
    -
    -          // TSEsPerNode is 0 for arkcmps started by the seapilot universal comsumers
    -          // in this case we only consider cpu cores
    -        if ((coresPerNode <= TSEsPerNode) || (TSEsPerNode == 0))
    -        {
    -            if (coresPerNode > 1)
    -                numESPsPerNode = DEFAULT_ESPS_PER_NODE; 
    -        }
    -        else if (coresPerNode > (TSEsPerNode*2))
    -        {
    -             numESPsPerNode = TSEsPerNode;
    -        }
    -        else if (TSEsPerNode > 1)
    -        {
    -             numESPsPerNode = TSEsPerNode/2;
    -        }
    -        else // not really needed since numESPsPerNode is set to 1 from above
    -        {
    -             numESPsPerNode = DEFAULT_ESPS_PER_NODE;
    -        }
    -        
    -
    -        ftoa_((float)(numESPsPerNode)/(float)(coresPerNode), valuestr);
    +        float espsPerCore = computeNumESPsPerCore(FALSE);
    --- End diff --
    
    I see.  I think one section of code was missing. My mistake.
    
    Please review the updated version. Sorry.
    
    
    
    On Thu, Jan 7, 2016 at 12:53 PM, DaveBirdsall <no...@github.com>
    wrote:
    
    > In core/sql/sqlcomp/nadefaults.cpp
    > <https://github.com/apache/incubator-trafodion/pull/250#discussion_r49110123>
    > :
    >
    > > -        else if (coresPerNode > (TSEsPerNode*2))
    > > -        {
    > > -             numESPsPerNode = TSEsPerNode;
    > > -        }
    > > -        else if (TSEsPerNode > 1)
    > > -        {
    > > -             numESPsPerNode = TSEsPerNode/2;
    > > -        }
    > > -        else // not really needed since numESPsPerNode is set to 1 from above
    > > -        {
    > > -             numESPsPerNode = DEFAULT_ESPS_PER_NODE;
    > > -        }
    > > -
    > > -
    > > -        ftoa_((float)(numESPsPerNode)/(float)(coresPerNode), valuestr);
    > > +        float espsPerCore = computeNumESPsPerCore(FALSE);
    >
    > Not sure I understand. We pass FALSE here to computeNumESPsPerCore, and
    > when I read that method, it goes to the else case (that is, !aggressive).
    > So it appears "not aggressive" is currently hard-coded?
    >
    > —
    > Reply to this email directly or view it on GitHub
    > <https://github.com/apache/incubator-trafodion/pull/250/files#r49110123>.
    >
    
    
    
    -- 
    Regards, --Qifan



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: fix JIRA-1739 to aggressively al...

Posted by zellerh <gi...@git.apache.org>.
Github user zellerh commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/250#discussion_r49024511
  
    --- Diff: core/sql/sqlcomp/nadefaults.cpp ---
    @@ -5914,6 +5866,73 @@ enum DefaultConstants NADefaults::validateAndInsert(const char *attrName,
     
     } // NADefaults::validateAndInsert()
     
    +float NADefaults::computeNumESPsPerCore(NABoolean aggressive)
    +{
    +   #define DEFAULT_ESPS_PER_NODE 2   // for conservation allocation
    +   #define DEFAULT_ESPS_PER_CORE 0.5 // for aggressive allocation
    +
    +     // Make sure the gpClusterInfo points at an NAClusterLinux object.
    +     // In osim simulation mode, the pointer can point at a NAClusterNSK
    +     // object, for which the method numTSEsForPOS() is not defined.
    +   NAClusterInfoLinux* gpLinux = dynamic_cast<NAClusterInfoLinux*>(gpClusterInfo);
    +   assert(gpLinux);		
    +
    +   // cores per node
    +   Lng32 coresPerNode = gpClusterInfo->numberOfCpusPerSMP();
    +
    +   if ( aggressive ) {
    +      float totalMemory = gpLinux->totalMemoryAvailable(); // per Node, in KB
    +      totalMemory /= 1024; // per Node, in GB
    +      totalMemory /= coresPerNode ; // per core, in GB
    +      totalMemory /= 2; // per core, 2GB per ESP
    +      return MINOF(DEFAULT_ESPS_PER_CORE, totalMemory);
    +   }
    +
    +   Lng32 numESPsPerNode = DEFAULT_ESPS_PER_NODE;
    +
    +     // number of POS TSE
    +   Lng32 numTSEsPerCluster = gpLinux->numTSEsForPOS();
    +
    +     // cluster nodes
    +   Lng32 nodesdPerCluster = gpClusterInfo->getTotalNumberOfCPUs();
    +
    +     // TSEs per node
    +   Lng32 TSEsPerNode = numTSEsPerCluster/nodesdPerCluster;
    +
    +
    +
    +     // For Linux/nt, we conservatively allocate ESPs per node as follows
    +     // - 1 ESP per 2 cpu cores if cores are equal or less than TSEs
    +     // - 1 ESP per TSE if number of cores is more than double the TSEs
    +     // - 1 ESP per 2 TSEs if cores are more than TSEs but less than double the TSEs
    +     // - 1 ESP per node. Only possible on NT or workstations
    +     //      - number of cores less than TSEs and there are 1 or 2 cpur cores per node
    +     //      - number of TSEs is less than cpu cores and there 1 or 2 TSEs per node.
    +     //        This case is probable if virtual nodes are used
    +
    +     // TSEsPerNode is 0 for arkcmps started by the seapilot universal comsumers
    +     // in this case we only consider cpu cores
    +   if ( coresPerNode <= TSEsPerNode || TSEsPerNode == 0 )
    +   {
    +       if (coresPerNode > 1)
    +           numESPsPerNode = DEFAULT_ESPS_PER_NODE; 
    +   }
    +   else if (coresPerNode > (TSEsPerNode*2))
    +   {
    +        numESPsPerNode = TSEsPerNode;
    +   }
    +   else if (TSEsPerNode > 1)
    +   {
    +        numESPsPerNode = TSEsPerNode/2;
    +   }
    +   else // not really needed since numESPsPerNode is set to 1 from above
    +   {
    +        numESPsPerNode = DEFAULT_ESPS_PER_NODE;
    +   }
    +        
    +   return (float)(numESPsPerNode)/(float)(coresPerNode);
    --- End diff --
    
    Isn't this a bit weird? We base the whole computation on TSEsPerNode, which is 0 on Trafodion (?), so if my assumption is true then all these conditions do nothing and we go with numESPsPerNode = 2? In other words, we completely ignore how many cores we have and only allow 2 ESPs per node?
    
    Hopefully we can override all this with the NUM_ESPS_PER_CPU_PER_OP CQD?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: fix JIRA-1739 to aggressively al...

Posted by zellerh <gi...@git.apache.org>.
Github user zellerh commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/250#discussion_r49034958
  
    --- Diff: core/sql/sqlcomp/nadefaults.cpp ---
    @@ -5914,6 +5866,73 @@ enum DefaultConstants NADefaults::validateAndInsert(const char *attrName,
     
     } // NADefaults::validateAndInsert()
     
    +float NADefaults::computeNumESPsPerCore(NABoolean aggressive)
    +{
    +   #define DEFAULT_ESPS_PER_NODE 2   // for conservation allocation
    +   #define DEFAULT_ESPS_PER_CORE 0.5 // for aggressive allocation
    +
    +     // Make sure the gpClusterInfo points at an NAClusterLinux object.
    +     // In osim simulation mode, the pointer can point at a NAClusterNSK
    +     // object, for which the method numTSEsForPOS() is not defined.
    +   NAClusterInfoLinux* gpLinux = dynamic_cast<NAClusterInfoLinux*>(gpClusterInfo);
    +   assert(gpLinux);		
    +
    +   // cores per node
    +   Lng32 coresPerNode = gpClusterInfo->numberOfCpusPerSMP();
    +
    +   if ( aggressive ) {
    +      float totalMemory = gpLinux->totalMemoryAvailable(); // per Node, in KB
    --- End diff --
    
    My comment is more that right now it's hard-coded to non-aggressive as far as I can see. Don't you want to have a CQD to turn it on and off - since you added a CQD. But, as far as I can see, the new CQD does nothing right now?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: fix JIRA-1739 to aggressively al...

Posted by nonstop-qfchen <gi...@git.apache.org>.
Github user nonstop-qfchen commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/250#discussion_r49039017
  
    --- Diff: core/sql/sqlcomp/nadefaults.cpp ---
    @@ -5914,6 +5866,73 @@ enum DefaultConstants NADefaults::validateAndInsert(const char *attrName,
     
     } // NADefaults::validateAndInsert()
     
    +float NADefaults::computeNumESPsPerCore(NABoolean aggressive)
    +{
    +   #define DEFAULT_ESPS_PER_NODE 2   // for conservation allocation
    +   #define DEFAULT_ESPS_PER_CORE 0.5 // for aggressive allocation
    +
    +     // Make sure the gpClusterInfo points at an NAClusterLinux object.
    +     // In osim simulation mode, the pointer can point at a NAClusterNSK
    +     // object, for which the method numTSEsForPOS() is not defined.
    +   NAClusterInfoLinux* gpLinux = dynamic_cast<NAClusterInfoLinux*>(gpClusterInfo);
    +   assert(gpLinux);		
    +
    +   // cores per node
    +   Lng32 coresPerNode = gpClusterInfo->numberOfCpusPerSMP();
    +
    +   if ( aggressive ) {
    +      float totalMemory = gpLinux->totalMemoryAvailable(); // per Node, in KB
    +      totalMemory /= 1024; // per Node, in GB
    +      totalMemory /= coresPerNode ; // per core, in GB
    +      totalMemory /= 2; // per core, 2GB per ESP
    +      return MINOF(DEFAULT_ESPS_PER_CORE, totalMemory);
    +   }
    +
    +   Lng32 numESPsPerNode = DEFAULT_ESPS_PER_NODE;
    +
    +     // number of POS TSE
    +   Lng32 numTSEsPerCluster = gpLinux->numTSEsForPOS();
    --- End diff --
    
    The complex code is commented out and an ELSE branch is added to
    effectively return 2 ESP per node for non-aggressive allocation scheme.
    
    The aggressive mode will be turned on after performance testing.
    
    On Wed, Jan 6, 2016 at 8:04 PM, Hans Zeller <no...@github.com>
    wrote:
    
    > In core/sql/sqlcomp/nadefaults.cpp
    > <https://github.com/apache/incubator-trafodion/pull/250#discussion_r49035086>
    > :
    >
    > > +
    > > +   // cores per node
    > > +   Lng32 coresPerNode = gpClusterInfo->numberOfCpusPerSMP();
    > > +
    > > +   if ( aggressive ) {
    > > +      float totalMemory = gpLinux->totalMemoryAvailable(); // per Node, in KB
    > > +      totalMemory /= 1024; // per Node, in GB
    > > +      totalMemory /= coresPerNode ; // per core, in GB
    > > +      totalMemory /= 2; // per core, 2GB per ESP
    > > +      return MINOF(DEFAULT_ESPS_PER_CORE, totalMemory);
    > > +   }
    > > +
    > > +   Lng32 numESPsPerNode = DEFAULT_ESPS_PER_NODE;
    > > +
    > > +     // number of POS TSE
    > > +   Lng32 numTSEsPerCluster = gpLinux->numTSEsForPOS();
    >
    > My vote is for getting rid of the code, it's confusing. As you say, it's
    > probably not very useful anyway. Why don't we just use
    > MAX_ESPS_PER_CPU_PER_OP (default value of 0.5) for the non-aggressive
    > method?
    >
    > —
    > Reply to this email directly or view it on GitHub
    > <https://github.com/apache/incubator-trafodion/pull/250/files#r49035086>.
    >
    
    
    
    -- 
    Regards, --Qifan



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: fix JIRA-1739 to aggressively al...

Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/250#discussion_r49110123
  
    --- Diff: core/sql/sqlcomp/nadefaults.cpp ---
    @@ -4333,59 +4336,8 @@ void NADefaults::updateSystemParameters(NABoolean reInit)
     
         case MAX_ESPS_PER_CPU_PER_OP:
           {
    -        // set 2 ESPs per node, as a starting point.
    -        #define DEFAULT_ESPS_PER_NODE 2
    -
    -        Lng32 numESPsPerNode = DEFAULT_ESPS_PER_NODE;
    -        Lng32 coresPerNode = 1;        
    -          // Make sure the gpClusterInfo points at an NAClusterLinux object.
    -          // In osim simulation mode, the pointer can point at a NAClusterNSK
    -          // object, for which the method numTSEsForPOS() is not defined.
    -        NAClusterInfoLinux* gpLinux = dynamic_cast<NAClusterInfoLinux*>(gpClusterInfo);
    -
    -          // number of POS TSE
    -        Lng32 numTSEsPerCluster = gpLinux->numTSEsForPOS();
    -
    -          // cluster nodes
    -        Lng32 nodesdPerCluster = gpClusterInfo->getTotalNumberOfCPUs();
    -
    -          // TSEs per node
    -        Lng32 TSEsPerNode = numTSEsPerCluster/nodesdPerCluster;
    -
    -          // cores per node
    -        coresPerNode = gpClusterInfo->numberOfCpusPerSMP();
    -
    -          // For Linux/nt, we conservatively allocate ESPs per node as follows
    -          // - 1 ESP per 2 cpu cores if cores are equal or less than TSEs
    -          // - 1 ESP per TSE if number of cores is more than double the TSEs
    -          // - 1 ESP per 2 TSEs if cores are more than TSEs but less than double the TSEs
    -          // - 1 ESP per node. Only possible on NT or workstations
    -          //      - number of cores less than TSEs and there are 1 or 2 cpur cores per node
    -          //      - number of TSEs is less than cpu cores and there 1 or 2 TSEs per node.
    -          //        This case is probable if virtual nodes are used
    -
    -          // TSEsPerNode is 0 for arkcmps started by the seapilot universal comsumers
    -          // in this case we only consider cpu cores
    -        if ((coresPerNode <= TSEsPerNode) || (TSEsPerNode == 0))
    -        {
    -            if (coresPerNode > 1)
    -                numESPsPerNode = DEFAULT_ESPS_PER_NODE; 
    -        }
    -        else if (coresPerNode > (TSEsPerNode*2))
    -        {
    -             numESPsPerNode = TSEsPerNode;
    -        }
    -        else if (TSEsPerNode > 1)
    -        {
    -             numESPsPerNode = TSEsPerNode/2;
    -        }
    -        else // not really needed since numESPsPerNode is set to 1 from above
    -        {
    -             numESPsPerNode = DEFAULT_ESPS_PER_NODE;
    -        }
    -        
    -
    -        ftoa_((float)(numESPsPerNode)/(float)(coresPerNode), valuestr);
    +        float espsPerCore = computeNumESPsPerCore(FALSE);
    --- End diff --
    
    Not sure I understand. We pass FALSE here to computeNumESPsPerCore, and when I read that method, it goes to the else case (that is, !aggressive). So it appears "not aggressive" is currently hard-coded?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: fix JIRA-1739 to aggressively al...

Posted by zellerh <gi...@git.apache.org>.
Github user zellerh commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/250#discussion_r49023568
  
    --- Diff: core/sql/sqlcomp/nadefaults.cpp ---
    @@ -5914,6 +5866,73 @@ enum DefaultConstants NADefaults::validateAndInsert(const char *attrName,
     
     } // NADefaults::validateAndInsert()
     
    +float NADefaults::computeNumESPsPerCore(NABoolean aggressive)
    +{
    +   #define DEFAULT_ESPS_PER_NODE 2   // for conservation allocation
    +   #define DEFAULT_ESPS_PER_CORE 0.5 // for aggressive allocation
    +
    +     // Make sure the gpClusterInfo points at an NAClusterLinux object.
    +     // In osim simulation mode, the pointer can point at a NAClusterNSK
    +     // object, for which the method numTSEsForPOS() is not defined.
    +   NAClusterInfoLinux* gpLinux = dynamic_cast<NAClusterInfoLinux*>(gpClusterInfo);
    +   assert(gpLinux);		
    +
    +   // cores per node
    +   Lng32 coresPerNode = gpClusterInfo->numberOfCpusPerSMP();
    +
    +   if ( aggressive ) {
    +      float totalMemory = gpLinux->totalMemoryAvailable(); // per Node, in KB
    --- End diff --
    
    I can't see how we would reach this code right now, since the only call to this method (on line 4339) passes FALSE for aggressive. Is that ok?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---