Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added querycount to getWithExtraInfo #367

Merged
merged 3 commits into from
Feb 28, 2024
Merged

Conversation

rakhiagr
Copy link
Contributor

@rakhiagr rakhiagr commented Feb 27, 2024

Summary

Fixed the getWithExtraInfo which was not using pagination, which was creating queries with 5000 Union statements. Also set the default value of queryCount as 50, and set the limitation on the setter method to not set above 50.

In conversation with MySQL team to know the ideal pagination count, they suggested 12. 12 seems too low looking at the 5000 union ran today. Working with them to test it on test server or prod read-replica

Update:

MySQL team suggested to have any number between 12-100, it's all guesses and it's difficult to get the exact one without proper metrics. Yang suggested to go with 50 for now, but we can change it one way or the other.

Testing

./gradlew build

Tested locally on metadata-store

Made few changes to TMS:

  • Added dao.setQueryKeysCount(2); to BaseLocalDaoFactory
  • Commented dao.setQueryKeysCount(cfg.queryKeysCount); in DatasetLocalDaoFactory

Ingested some dummy records

curli "http://localhost:1472/datasets?action=ingestWithTracking" 
-H 'X-RestLi-Method:action' -H 'Accept:application/json' 
-H 'Content-Type:application/json' -H 'X-RestLi-Protocol-Version:2.0.0' 
--data '{
     "snapshot": {
         "urn": "urn:li:dataset:(urn:li:dataPlatform:hdfs,rakagraw_test1,EI)",
         "aspects": [
             {
                 "com.linkedin.common.Status": {
                     "removed": false
                 }
             }
         ]
     },
     "trackingContext": {
         "emitter": "urn:li:corpuser:tester",
         "emitTime": 1708479115661
     }
 },{
     "snapshot": {
         "urn": "urn:li:dataset:(urn:li:dataPlatform:hdfs,rakagraw_test2,EI)",
         "aspects": [
             {
                 "com.linkedin.common.Status": {
                     "removed": false
                 }
             }
         ]
     },
     "trackingContext": {
         "emitter": "urn:li:corpuser:tester",
         "emitTime": 1708479115662
     }
 },
{
     "snapshot": {
         "urn": "urn:li:dataset:(urn:li:dataPlatform:hdfs,rakagraw_test3,EI)",
         "aspects": [
             {
                 "com.linkedin.common.Status": {
                     "removed": false
                 }
             }
         ]
     },
     "trackingContext": {
         "emitter": "urn:li:corpuser:tester",
         "emitTime": 1708479115663
     }
 }'
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100   757    0     0  100   757      0   6060 --:--:-- --:--:-- --:--:--  6104

Run batchGet to get Status aspect for the dummy records

 curli "http://localhost:1472/datasets?
aspects=List(com.linkedin.common.Status)&
ids=List((\$params:(),name:rakagraw_test2,origin:EI,platform:urn%3Ali%3AdataPlatform%3Ahdfs),
(\$params:(),name:rakagraw_test1,origin:EI,platform:urn%3Ali%3AdataPlatform%3Ahdfs),
(\$params:(),name:rakagraw_test3,origin:EI,platform:urn%3Ali%3AdataPlatform%3Ahdfs))" 
-X GET 
 -H 'Accept:application/json' -H 'Content-Type:application/json' -H 'X-RestLi-Protocol-Version:2.0.0'
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  1484  100  1484    0     0  26269      0 --:--:-- --:--:-- --:--:-- 26500
{
    "statuses": {},
    "results": {
        "(name:rakagraw_test2,origin:EI,platform:urn%3Ali%3AdataPlatform%3Ahdfs)": {
            "removed": false,
            "created": {
                "actor": "urn:li:corpuser:UNKNOWN",
                "time": 1709082679047
            },
            "origin": "EI",
            "name": "rakagraw_test2",
            "description": "",
            "lastModified": {
                "actor": "urn:li:corpuser:UNKNOWN",
                "time": 1709082679047
            },
            "id": 1,
            "deploymentInfos": [
                {
                    "dataLocation": {
                        "cluster": "default",
                        "fabricGroup": "EI"
                    }
                }
            ],
            "$URN": "urn:li:dataset:(urn:li:dataPlatform:hdfs,rakagraw_test2,EI)",
            "platform": "urn:li:dataPlatform:hdfs",
            "tags": [],
            "status": {
                "removed": false
            }
        },
        "(name:rakagraw_test3,origin:EI,platform:urn%3Ali%3AdataPlatform%3Ahdfs)": {
            "created": {
                "actor": "urn:li:corpuser:UNKNOWN",
                "time": 1709082679039
            },
            "origin": "EI",
            "name": "rakagraw_test3",
            "description": "",
            "lastModified": {
                "actor": "urn:li:corpuser:UNKNOWN",
                "time": 1709082679039
            },
            "id": 1,
            "deploymentInfos": [],
            "$URN": "urn:li:dataset:(urn:li:dataPlatform:hdfs,rakagraw_test3,EI)",
            "platform": "urn:li:dataPlatform:hdfs",
            "tags": []
        },
        "(name:rakagraw_test1,origin:EI,platform:urn%3Ali%3AdataPlatform%3Ahdfs)": {
            "removed": false,
            "created": {
                "actor": "urn:li:corpuser:UNKNOWN",
                "time": 1709082679041
            },
            "origin": "EI",
            "name": "rakagraw_test1",
            "description": "",
            "lastModified": {
                "actor": "urn:li:corpuser:UNKNOWN",
                "time": 1709082679041
            },
            "id": 1,
            "deploymentInfos": [
                {
                    "dataLocation": {
                        "cluster": "default",
                        "fabricGroup": "EI"
                    }
                }
            ],
            "$URN": "urn:li:dataset:(urn:li:dataPlatform:hdfs,rakagraw_test1,EI)",
            "platform": "urn:li:dataPlatform:hdfs",
            "tags": [],
            "status": {
                "removed": false
            }
        }
    },
    "errors": {}
}

Checked the log on metagalaxy_local mysql.general_log table

Ran these two commands to enable general_log before running batch get

SET GLOBAL log_output = 'TABLE';
SET GLOBAL general_log = 'ON';

Now, checked the general_log to see UNION of size 2, please scroll right to argument column

MariaDB [metagalaxy_local]> select * from mysql.general_log where argument like 'SELECT urn%';
+----------------------------+--------------------------+-----------+-----------+--------------+-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| event_time                 | user_host                | thread_id | server_id | command_type | argument                                                                                                                                                                                                                                                                                                                                                                                                                                  |
+----------------------------+--------------------------+-----------+-----------+--------------+-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| 2024-02-27 17:11:19.031531 | [user] @ localhost [::1] |        31 |         1 | Query        | SELECT urn, a_status, lastmodifiedon, lastmodifiedby FROM metadata_entity_dataset WHERE urn = 'urn:li:dataset:(urn:li:dataPlatform:hdfs,rakagraw_test3,EI)' AND JSON_EXTRACT(a_status, '$.gma_deleted') IS NULL UNION ALL SELECT urn, a_status, lastmodifiedon, lastmodifiedby FROM metadata_entity_dataset WHERE urn = 'urn:li:dataset:(urn:li:dataPlatform:hdfs,rakagraw_test1,EI)' AND JSON_EXTRACT(a_status, '$.gma_deleted') IS NULL |
| 2024-02-27 17:11:19.038086 | [user] @ localhost [::1] |        31 |         1 | Query        | SELECT urn, a_status, lastmodifiedon, lastmodifiedby FROM metadata_entity_dataset WHERE urn = 'urn:li:dataset:(urn:li:dataPlatform:hdfs,rakagraw_test2,EI)' AND JSON_EXTRACT(a_status, '$.gma_deleted') IS NULL                                                                                                                                                                                                                           |
| 2024-02-27 17:11:19.040336 | [user] @ localhost [::1] |        31 |         1 | Query        | SELECT urn FROM metadata_entity_datasetinstance
WHERE i_urn$dataset = 'urn:li:dataset:(urn:li:dataPlatform:hdfs,rakagraw_test3,EI)'
ORDER BY URN LIMIT 1024                                                                                                                                                                                                                                                                               |
| 2024-02-27 17:11:19.041369 | [user] @ localhost [::1] |        31 |         1 | Query        | SELECT urn FROM metadata_entity_datasetinstance
WHERE i_urn$dataset = 'urn:li:dataset:(urn:li:dataPlatform:hdfs,rakagraw_test1,EI)'
ORDER BY URN LIMIT 1024                                                                                                                                                                                                                                                                               |
| 2024-02-27 17:11:19.043222 | [user] @ localhost [::1] |        31 |         1 | Query        | SELECT urn, a_status, lastmodifiedon, lastmodifiedby FROM metadata_entity_datasetinstance WHERE urn = 'urn:li:datasetInstance:(urn:li:dataset:(urn:li:dataPlatform:hdfs,rakagraw_test1,EI),default)' AND JSON_EXTRACT(a_status, '$.gma_deleted') IS NULL                                                                                                                                                                                  |
| 2024-02-27 17:11:19.046011 | [user] @ localhost [::1] |        31 |         1 | Query        | SELECT urn, a_deploymentinfo, lastmodifiedon, lastmodifiedby FROM metadata_entity_datasetinstance WHERE urn = 'urn:li:datasetInstance:(urn:li:dataset:(urn:li:dataPlatform:hdfs,rakagraw_test1,EI),default)' AND JSON_EXTRACT(a_deploymentinfo, '$.gma_deleted') IS NULL                                                                                                                                                                  |
| 2024-02-27 17:11:19.048432 | [user] @ localhost [::1] |        31 |         1 | Query        | SELECT urn FROM metadata_entity_datasetinstance
WHERE i_urn$dataset = 'urn:li:dataset:(urn:li:dataPlatform:hdfs,rakagraw_test2,EI)'
ORDER BY URN LIMIT 1024                                                                                                                                                                                                                                                                               |
| 2024-02-27 17:11:19.050202 | [user] @ localhost [::1] |        31 |         1 | Query        | SELECT urn, a_status, lastmodifiedon, lastmodifiedby FROM metadata_entity_datasetinstance WHERE urn = 'urn:li:datasetInstance:(urn:li:dataset:(urn:li:dataPlatform:hdfs,rakagraw_test2,EI),default)' AND JSON_EXTRACT(a_status, '$.gma_deleted') IS NULL                                                                                                                                                                                  |
| 2024-02-27 17:11:19.051668 | [user] @ localhost [::1] |        31 |         1 | Query        | SELECT urn, a_deploymentinfo, lastmodifiedon, lastmodifiedby FROM metadata_entity_datasetinstance WHERE urn = 'urn:li:datasetInstance:(urn:li:dataset:(urn:li:dataPlatform:hdfs,rakagraw_test2,EI),default)' AND JSON_EXTRACT(a_deploymentinfo, '$.gma_deleted') IS NULL                                                                                                                                                                  |
+----------------------------+--------------------------+-----------+-----------+--------------+-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
9 rows in set (0.002 sec)



Checklist

  • The PR conforms to DataHub's Contributing Guideline (particularly Commit Message Format)
  • Links to related issues (if applicable)
  • Tests for the changes have been added/updated (if applicable)
  • Docs related to the changes have been added/updated (if applicable)

@yangyangv2
Copy link
Contributor

Instead of trunking the response, which will change the existing behavior, we should truncate large request into small trunks, process them in multiple requests and then merge the results together.

@rakhiagr
Copy link
Contributor Author

Instead of trunking the response, which will change the existing behavior, we should truncate large request into small trunks, process them in multiple requests and then merge the results together.

@yangyangv2 Isn't this what we are doing here?

Copy link
Contributor

@jsdonn jsdonn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but please make sure Yang is on the same page before merging.

@@ -79,7 +79,7 @@ public class EbeanLocalDAO<ASPECT_UNION extends UnionTemplate, URN extends Urn>

protected final EbeanServer _server;
protected final Class<URN> _urnClass;
private int _queryKeysCount = 0; // 0 means no pagination on keys
private int _queryKeysCount = 100;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

create a new variable and assign it to here

private int final static DEFAULT_BATCH_SIZE = 50;
private int  _queryKeysCount = DEFAULT_BATCH_SIZE;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

100 "UNION ALL" is still significant for queuing large tables. Let's use 50 as a short-term fix.

The longer time fix should be replacing "UNION ALL" with "IN" clause

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The longer time fix should be replacing "UNION ALL" with "IN" clause

I can keep it as scope of my reliability project.

@@ -947,6 +950,10 @@ public void setQueryKeysCount(int keysCount) {
if (keysCount < 0) {
throw new IllegalArgumentException("Query keys count must be non-negative: " + keysCount);
}
if (keysCount > 100) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use DEFAULT_BATCH_SIZE in line 953~955

@yangyangv2
Copy link
Contributor

Overall looks good, thanks for including the testing examples!

Copy link
Contributor

@yangyangv2 yangyangv2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for addressing the comments!

@rakhiagr rakhiagr merged commit 0ebf2ce into master Feb 28, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants