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

[core] Moved namespaces and computes to database #3446

Merged

Conversation

amitsrivastava
Copy link
Collaborator

@amitsrivastava amitsrivastava commented Aug 25, 2023

The existing implementation was connecting to external Altus and DWX
api's to fetch the available computes. This commit changes that by
moving the cluster config to database. Two new tables have been added
(a) beeswax_namespace to hold config for a namespace/dialect
(b) beeswax_compute to hold configs for individual compute clusters
linked to the namespaces.
This change currently support hive and impala clusters.

There is also a service-discovery component that keeps the list of
namespaces and computes updated in the corresponding tables.
sync_warehouses.py performs the service discovery in the CDW environ
by talking to kubernetes api's. It creates one Hive and one Impala
namespace and one compute for each virtual warehouse. The command is
supposed to run every minute and keep the list of warehouses updated.

There are related changes and fixes to the rest of the code to support
the query execution on different computes.

Change-Id: Ifd8dbc8d716dfe2000fbfa8121e39f2610051fa1

Copy link
Contributor

@ranade1 ranade1 left a comment

Choose a reason for hiding this comment

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

looks good so far. if I get some code walkthrough will be helpful.

apps/beeswax/src/beeswax/server/dbms.py Outdated Show resolved Hide resolved
@@ -0,0 +1,203 @@
#!/usr/bin/env python
Copy link
Contributor

Choose a reason for hiding this comment

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

how frequently does the sync warehouse happen? Is this script responsible for deleting VW which has gone away?

Copy link
Collaborator

@wing2fly wing2fly left a comment

Choose a reason for hiding this comment

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

Please do downgrade test before merging.

Copy link
Collaborator

@Harshg999 Harshg999 left a comment

Choose a reason for hiding this comment

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

Nice work! Taking shape, few review comments to address.

apps/beeswax/src/beeswax/server/dbms.py Outdated Show resolved Hide resolved
apps/beeswax/src/beeswax/server/dbms.py Show resolved Hide resolved
apps/beeswax/src/beeswax/server/dbms.py Show resolved Hide resolved
apps/jobbrowser/src/jobbrowser/apis/query_api.py Outdated Show resolved Hide resolved

response[interface] = namespaces
response['status'] = 0

return JsonResponse(response)


@api_error_handler
def get_context_computes(request, interface):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Update docstring

desktop/core/src/desktop/lib/computes/models.py Outdated Show resolved Hide resolved
desktop/core/src/desktop/lib/computes/models.py Outdated Show resolved Hide resolved
@@ -1800,7 +1800,7 @@ def get_config(self):
],
'default_sql_interpreter': default_sql_interpreter,
'cluster_type': self.cluster_type,
'has_computes': self.cluster_type in ('altus', 'snowball'), # or any grouped engine connectors
'has_computes': self.cluster_type in ('cdw', 'altus', 'snowball'), # or any grouped engine connectors
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's remove altus and snowball? They are not used anywhere right?
@JohanAhlen Any idea?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am thinking of creating a separate commit to remove the altus/snowball related code.

@amitsrivastava amitsrivastava force-pushed the dev/amit/compute-with-namespace-master-aug-25 branch 2 times, most recently from f81856f to 074dae5 Compare September 14, 2023 19:34
@amitsrivastava
Copy link
Collaborator Author

Please do downgrade test before merging.

Done the downgrade test. Rollback worked fine.

@amitsrivastava amitsrivastava force-pushed the dev/amit/compute-with-namespace-master-aug-25 branch 5 times, most recently from 7a1c082 to 3ea8782 Compare September 15, 2023 20:32
Copy link
Contributor

@ranade1 ranade1 left a comment

Choose a reason for hiding this comment

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

Looks good

@amitsrivastava amitsrivastava force-pushed the dev/amit/compute-with-namespace-master-aug-25 branch from 3ea8782 to 7b326ed Compare September 18, 2023 22:13
@amitsrivastava amitsrivastava enabled auto-merge (rebase) September 18, 2023 22:14
@amitsrivastava amitsrivastava force-pushed the dev/amit/compute-with-namespace-master-aug-25 branch 3 times, most recently from 3c9a591 to acaeb5f Compare September 19, 2023 05:23
The existing implementation was connecting to external Altus and DWX
api's to fetch the available computes. This commit changes that by
moving the cluster config to database. Two new tables have been added
(a) `beeswax_namespace` to hold config for a namespace/dialect
(b) `beeswax_compute` to hold configs for individual compute clusters
linked to the namespaces.
This change currently support hive and impala clusters.

There is also a service-discovery component that keeps the list of
namespaces and computes updated in the corresponding tables.
`sync_warehouses.py` performs the service discovery in the CDW environ
by talking to kubernetes api's. It creates one Hive and one Impala
namespace and one compute for each virtual warehouse. The command is
supposed to run every minute and keep the list of warehouses updated.

There are related changes and fixes to the rest of the code to support
the query execution on different computes.

Change-Id: Ifd8dbc8d716dfe2000fbfa8121e39f2610051fa1
@amitsrivastava amitsrivastava force-pushed the dev/amit/compute-with-namespace-master-aug-25 branch from acaeb5f to 3594a73 Compare September 19, 2023 05:50
@amitsrivastava amitsrivastava merged commit 283676c into master Sep 19, 2023
3 checks passed
@amitsrivastava amitsrivastava deleted the dev/amit/compute-with-namespace-master-aug-25 branch September 19, 2023 06:09
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.

4 participants