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.

Sign up for GitHub

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

Jump to bottom

[CDAP-20982] Removing Hbase code references as a part of Hadoop upgrade 2 to 3 #15595

Merged
merged 5 commits into from
May 16, 2024
Merged

[CDAP-20982] Removing Hbase code references as a part of Hadoop upgrade 2 to 3 #15595

merged 5 commits into from
May 16, 2024

Conversation

sahusanket
Copy link
Contributor

No description provided.

@sahusanket sahusanket self-assigned this Apr 15, 2024
@sahusanket sahusanket added the build Triggers github actions build label Apr 15, 2024
@sahusanket sahusanket marked this pull request as ready for review May 8, 2024 10:27
@sahusanket
Copy link
Contributor Author

Hi @chtyim ,
Please review this PR for removing hbase

  • I have tried to replace hbase with levelDB for some implementations and injections.

  • Removed hbase references in JAVA code.

  • There are few parts i am not sure : i.e. there are parts were only hbase is being used.

    • cdap-common/bin/functions.sh
    • cdap-master/src/main/java/io/cdap/cdap/data/runtime/main/MasterServiceMain.java
    • I have removed hbase refs but i am not sure if we still support the above usecases ? like cdap scripts ?

Testing :

  • Build sandbox, and ran basic operations, connections, pipeline run etx..all successful

  • Ran cdap-ui e2e test on above sandbox, seems successful

  • Replaced image in a GKE cluster ( CDF ) and tested basic operations/runs and it worked successfully.

Please let me know are there any other testing that i need to perform to ensure everything is working fine.

@sahusanket sahusanket requested review from tivv and chtyim May 8, 2024 11:26
tivv
tivv reviewed May 8, 2024
View reviewed changes
Copy link
Contributor

@tivv tivv left a comment

Choose a reason for hiding this comment

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

I see that cdap-data-fabric-tests failed. Could you check it?

cdap-common/src/main/java/io/cdap/cdap/common/messaging/MessagingUtils.java Outdated Show resolved Hide resolved
@sahusanket sahusanket requested a review from tivv May 11, 2024 16:22
tivv
tivv reviewed May 13, 2024
View reviewed changes
Copy link
Contributor

@tivv tivv left a comment

Choose a reason for hiding this comment

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

Also please update PR title to include JIRA ID and remove draft reference

...-fabric/src/main/java/io/cdap/cdap/internal/app/runtime/batch/BasicMapReduceTaskContext.java Outdated
@@ -383,11 +382,12 @@ public <T extends Dataset> T getDataset(String namespace, String name,
* true.
*/
private Map<String, String> adjustRuntimeArguments(Map<String, String> args) {
if (args.containsKey(HBaseTable.SAFE_INCREMENTS)) {
String SAFE_INCREMENTS = "dataset.table.safe.readless.increments";
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this was use only in HBaseTable, we can remove it altogether

...s/src/test/java/io/cdap/cdap/data2/transaction/distributed/TransactionServiceClientTest.java Outdated
hBaseTestingUtility.startMiniDFSCluster(1);
Configuration hConf = hBaseTestingUtility.getConfiguration();
Configuration hConf = new Configuration();
hConf.set(MiniDFSCluster.HDFS_MINIDFS_BASEDIR,tmpFolder.newFolder().getAbsolutePath());
Copy link
Contributor

Choose a reason for hiding this comment

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

Please fix checkstyle

...c-tests/src/test/java/io/cdap/cdap/data2/transaction/distributed/TransactionServiceTest.java Outdated
@@ -50,10 +50,12 @@
import io.cdap.cdap.spi.metadata.noop.NoopMetadataStorage;
import java.io.File;
import java.util.Arrays;
import java.util.Map;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please fix imports

Copy link
Contributor

Choose a reason for hiding this comment

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

Also please fix other checkstyle inssues in new code

pom.xml Outdated
@@ -2039,7 +1828,8 @@
<package.rpm.arch>all</package.rpm.arch>
<package.dirs>opt etc</package.dirs>
<package.config.dirs>--config-files etc/cdap --config-files etc/logrotate.d --config-files etc/security</package.config.dirs>
<dependency.groupId.exclusions>org.apache.hadoop,org.apache.hbase,\
<dependency.groupId.exclusions>org.apache.hadoop,\
<!-- org.apache.hbase,\-->
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't add commented code

@sahusanket sahusanket changed the title First draft - removing hbase - success compile [CDAP-20982] Removing Hbase code references as a part of Hadoop upgrade 2 to 3 May 14, 2024
@sahusanket sahusanket merged commit f05eb25 into develop May 16, 2024
11 of 12 checks passed
@sahusanket sahusanket deleted the CDAP-20982_remove_hbase branch May 16, 2024 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tivv tivv tivv approved these changes

@chtyim chtyim Awaiting requested review from chtyim

Assignees

@sahusanket sahusanket

Labels
build Triggers github actions build
Projects
None yet
Milestone
No milestone
2 participants
@sahusanket @tivv