feat(warehouse): glue partitions #2899

achettyiitr · 2023-01-24T08:12:01Z

Description

Glue partitions

AWS Glue provides partitioning support. Once the load files are generated we are refreshing the partitions.
We are calling refresh partitions after the loading of the data is completed since we have a dependency on schema, table, and columns getting created.

Reference: #2476

Notion Ticket

https://www.notion.so/rudderstacks/AWS-Glue-partitioning-b80301553aed40958df49c3a9beeba40

Security

The code changed/added as part of this pull request won't create any security issues with how the software is being used.

codecov · 2023-01-24T09:59:18Z

Codecov Report

Base: 51.33% // Head: 51.73% // Increases project coverage by +0.40% 🎉

Coverage data is based on head (63fcc30) compared to base (d049f2b).
Patch coverage: 65.24% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2899      +/-   ##
==========================================
+ Coverage   51.33%   51.73%   +0.40%     
==========================================
  Files         321      321              
  Lines       50646    50791     +145     
==========================================
+ Hits        25997    26279     +282     
+ Misses      23079    22931     -148     
- Partials     1570     1581      +11

Impacted Files	Coverage Δ
warehouse/upload.go	`21.48% <0.00%> (-0.44%)`	⬇️
warehouse/utils/utils.go	`70.07% <ø> (-0.90%)`	⬇️
...e/integrations/datalake/schema-repository/local.go	`89.28% <63.63%> (+89.28%)`	⬆️
...se/integrations/datalake/schema-repository/glue.go	`77.32% <74.07%> (+77.32%)`	⬆️
...ns/datalake/schema-repository/schema_repository.go	`77.27% <93.75%> (ø)`
warehouse/internal/loadfiles/loadfiles.go	`72.68% <95.23%> (+2.63%)`	⬆️
jobsdb/jobsdb.go	`73.95% <0.00%> (+0.12%)`	⬆️
utils/misc/misc.go	`49.15% <0.00%> (+0.67%)`	⬆️
warehouse/schema.go	`51.42% <0.00%> (+1.14%)`	⬆️
services/rsources/handler.go	`75.20% <0.00%> (+1.37%)`	⬆️
... and 4 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

warehouse/upload.go

warehouse/integrations/datalake/schema-repository/glue.go

…t.glue-partitions

achettyiitr · 2023-01-24T22:31:44Z

warehouse/integrations/datalake/schema-repository/local.go

-	if err != nil {
-		return err
-	}
+	schema, _, _ := ls.FetchSchema(ls.warehouse)


Since FetchSchema always returns nil, ignoring it.

~~Should we remove the err from FetchSchema then? We can re-introduce it once error is possible.~~

I see the schema needs to satisfy an interface.

One possible way to work around this:

func (ls *LocalSchemaRepository) localFetchSchema() warehouseutils.SchemaT { return ls.uploader.GetLocalSchema() } func (ls *LocalSchemaRepository) FetchSchema(_ warehouseutils.Warehouse) (warehouseutils.SchemaT, warehouseutils.SchemaT, error) { s := ls.localFetchSchema() return s, s, nil } func (ls *LocalSchemaRepository) CreateTable(tableName string, columnMap map[string]string) (err error) { schema:= ls.localFetchSchema(ls.warehouse)

achettyiitr · 2023-01-24T22:31:48Z

warehouse/integrations/datalake/schema-repository/local.go

-	if err != nil {
-		return err
-	}
+	schema, _, _ := ls.FetchSchema(ls.warehouse)


Since FetchSchema always returns nil, ignoring it.

achettyiitr · 2023-01-24T22:31:53Z

warehouse/integrations/datalake/schema-repository/local.go

-	if err != nil {
-		return err
-	}
+	schema, _, _ := ls.FetchSchema(ls.warehouse)


Since FetchSchema always returns nil, ignoring it.

warehouse/integrations/datalake/schema-repository/schema_repository.go

lvrach · 2023-01-25T08:09:43Z

warehouse/upload.go

@@ -482,6 +484,10 @@ func (job *UploadJobT) run() (err error) {
 			job.matchRowsInStagingAndLoadFiles()
 			job.recordLoadFileGenerationTimeStat(startLoadFileID, endLoadFileID)

+			if err = job.RefreshPartitions(startLoadFileID, endLoadFileID); err != nil {
+				break


Should we log the error? As an Warn possibly?

lvrach · 2023-01-25T08:16:41Z

warehouse/upload.go

+		// This is best done every 100 files, since it's a batch request for updates in Glue
+		partitionBatchSize := config.GetInt("Warehouse.refreshPartitionBatchSize", 99)
+		for i := 0; i < len(loadFiles); i += partitionBatchSize {
+			end := i + partitionBatchSize
+			if end > len(loadFiles) {
+				end = len(loadFiles)
+			}
+


Would it be better to move the batching logic to the GlueSchemaRepository or GlueClient ?

It seems destination-specific logic.

Also why GetInt("Warehouse.refreshPartitionBatchSize", 99) is not a 100 ?

There is a limit set by the SDK for the batching to be 100. Since these changes were contributed from an open source user who kept it to 99.
5c8a8c9

lvrach · 2023-01-25T09:15:35Z

warehouse/integrations/datalake/schema-repository/glue_test.go

+	uploadOutput, err := fm.Upload(context.TODO(), f, fmt.Sprintf("rudder-test-payload/s3-datalake/%s/dt=2006-01-02/", warehouseutils.RandHex()))
+	require.NoError(t, err)
+
+	err = g.RefreshPartitions(testTable, []warehouseutils.LoadFileT{


How are we checking the side effects of refresh partitions?

Currently, we have added only the round-trip flow for Glue. Since this requires an external service call to Glue. Should we mock the API call and tests this?

…t.glue-partitions

lvrach · 2023-01-31T07:53:40Z

warehouse/integrations/datalake/schema-repository/schema_repository.go

+	fileBatches := make([][]warehouseutils.LoadFileT, 0, len(files)/batchSize+1)
+
+	for len(files) > 0 {
+		cut := int(math.Min(float64(len(files)), float64(batchSize)))


In general would avoid using float64 for anything that requires precision: https://jvns.ca/blog/2023/01/13/examples-of-floating-point-problems/#example-1-the-odometer-that-stopped

However, I can not think of an example where this would fail. So take this as a minor suggestion:

Suggested change

cut := int(math.Min(float64(len(files)), float64(batchSize)))

cut := batchSize

if len(files) > cut {

cut = len(files)

}

Thanks for pointing that out. @lvrach

For some reason, I was expecting the math package to have func max(x, y int) int. Although there was one more package by Dave Cheney it's archived not. There is proposal to have a generic implementation for this.

achettyiitr added 2 commits January 24, 2023 13:08

added support for glue partitioning.

9ba80ee

GetLoadFilePrefix moved to loadfiles package.

7f2eb43

github-actions bot added warehouse-team with tests labels Jan 24, 2023

Jayachand reviewed Jan 24, 2023

View reviewed changes

warehouse/upload.go Show resolved Hide resolved

Jayachand reviewed Jan 24, 2023

View reviewed changes

warehouse/integrations/datalake/schema-repository/glue.go Outdated Show resolved Hide resolved

Jayachand approved these changes Jan 24, 2023

View reviewed changes

achettyiitr added 3 commits January 25, 2023 00:32

added test for schema respository.

7b89363

some more changes

2d8b6c4

Merge branch 'master' of github.com:rudderlabs/rudder-server into fea…

727ce75

…t.glue-partitions

achettyiitr marked this pull request as ready for review January 24, 2023 22:28

achettyiitr requested review from lvrach and abhimanyubabbar January 24, 2023 22:28

achettyiitr commented Jan 24, 2023

View reviewed changes

achettyiitr requested a review from Jayachand January 25, 2023 05:59

Jayachand reviewed Jan 25, 2023

View reviewed changes

warehouse/integrations/datalake/schema-repository/schema_repository.go Outdated Show resolved Hide resolved

lvrach reviewed Jan 25, 2023

View reviewed changes

achettyiitr added 3 commits January 25, 2023 13:56

handling window layout.

2d83d26

invalid file format

35fb007

logger for schema repository

57f5b15

achettyiitr requested review from Jayachand and sivashanmukh January 25, 2023 08:44

lvrach reviewed Jan 25, 2023

View reviewed changes

achettyiitr added 3 commits January 25, 2023 21:37

create parititions while refreshing

faeb322

Merge branch 'master' of github.com:rudderlabs/rudder-server into fea…

3b6c171

…t.glue-partitions

review comments.

b6ee12a

achettyiitr requested a review from lvrach January 29, 2023 12:56

achettyiitr added 3 commits January 29, 2023 18:29

lint issue resolved

ea841a9

moving refresh partitions to load table.

ea23e35

update test credentials

b4b3762

lvrach reviewed Jan 31, 2023

View reviewed changes

lvrach approved these changes Jan 31, 2023

View reviewed changes

achettyiitr added 2 commits January 31, 2023 14:19

review comments.

a4f3388

master pull

63fcc30

achettyiitr merged commit 9a928d4 into master Feb 1, 2023

achettyiitr deleted the feat.glue-partitions branch February 1, 2023 10:39

This was referenced Feb 7, 2023

chore: prerelease 1.6.0-alpha.1 #2954

Closed

chore: prerelease 1.6.0-preview.1 #2957

Merged

chore: prerelease 1.6.0-preview.2 #2962

Merged

chore: prerelease 1.6.0-preview.3 #2974

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(warehouse): glue partitions #2899

feat(warehouse): glue partitions #2899

achettyiitr commented Jan 24, 2023 •

edited

codecov bot commented Jan 24, 2023 •

edited

achettyiitr Jan 24, 2023

lvrach Jan 25, 2023 •

edited

achettyiitr Jan 24, 2023

achettyiitr Jan 24, 2023

lvrach Jan 25, 2023

lvrach Jan 25, 2023

achettyiitr Jan 29, 2023 •

edited

lvrach Jan 25, 2023

achettyiitr Jan 29, 2023

lvrach Jan 31, 2023

achettyiitr Jan 31, 2023

achettyiitr Jan 31, 2023

-		cut := int(math.Min(float64(len(files)), float64(batchSize)))
+		cut := batchSize
+		if len(files) > cut {
+		    cut = len(files)
+		}

feat(warehouse): glue partitions #2899

feat(warehouse): glue partitions #2899

Conversation

achettyiitr commented Jan 24, 2023 • edited

Description

Glue partitions

Notion Ticket

Security

codecov bot commented Jan 24, 2023 • edited

Codecov Report

Choose a reason for hiding this comment

lvrach Jan 25, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

achettyiitr Jan 29, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

achettyiitr commented Jan 24, 2023 •

edited

codecov bot commented Jan 24, 2023 •

edited

lvrach Jan 25, 2023 •

edited

achettyiitr Jan 29, 2023 •

edited