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

[fix][offload] use filesystem offloader with class not found error #20365

Merged

Conversation

ethqunzhong
Copy link
Contributor

Fixes #16955

Motivation

fix the problem about Failing to start standalone pulsar service with tiered storage due to class not found problem
The reason is the lack of dependencies for the org.apache.hadoop.hdfs.DistributedFileSystem class.

Modifications

add DistributedFileSystem class dependency hadoop-client in pom.xml

Verifying this change

  • Make sure that the change passes the CI checks.

This change is already covered by existing tests.

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository: ethqunzhong#5

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label May 21, 2023
@ethqunzhong
Copy link
Contributor Author

@eolivelli PTAL

@poorbarcode poorbarcode added this to the 3.1.0 milestone May 30, 2023
Copy link
Member

@tisonkun tisonkun left a comment

Choose a reason for hiding this comment

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

It seems the issued class is located at hadoop-hdfs-client. Could you elaborate a bit why hadoop-client is needed here?

@ethqunzhong ethqunzhong force-pushed the qunzhong-fix-hdfs-offload-class-notfound branch from 2ce2a9a to 3fb0e8f Compare June 15, 2023 09:49
@ethqunzhong
Copy link
Contributor Author

It seems the issued class is located at hadoop-hdfs-client. Could you elaborate a bit why hadoop-client is needed here?

@tisonkun Sorry for not timely reply. What you said is correct. After testing, simplifying the dependencies still works.

Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

+1

it would be great to setup an integration test one day.
I assume that you tested it manually

Is this problem also on Pulsar 3.0 ?
should we cherry-pick this PR to branch-3.0 ?

@codecov-commenter
Copy link

Codecov Report

Merging #20365 (3fb0e8f) into master (d85736c) will increase coverage by 0.07%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #20365      +/-   ##
============================================
+ Coverage     73.00%   73.07%   +0.07%     
- Complexity    31962    31991      +29     
============================================
  Files          1867     1867              
  Lines        138661   138661              
  Branches      15235    15235              
============================================
+ Hits         101227   101328     +101     
+ Misses        29382    29301      -81     
+ Partials       8052     8032      -20     
Flag Coverage Δ
inttests 24.13% <ø> (+0.04%) ⬆️
systests 24.97% <ø> (-0.08%) ⬇️
unittests 72.36% <ø> (+0.08%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 65 files with indirect coverage changes

Copy link
Member

@tisonkun tisonkun left a comment

Choose a reason for hiding this comment

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

Thank you!

@tisonkun tisonkun requested a review from hangc0276 June 15, 2023 15:19
@ethqunzhong
Copy link
Contributor Author

+1

it would be great to setup an integration test one day. I assume that you tested it manually

Is this problem also on Pulsar 3.0 ? should we cherry-pick this PR to branch-3.0 ?

yes. tested it manually and branch-3.0 has the same problem.

@tisonkun
Copy link
Member

Merging...

cc @RobertIndie I guess we can include this patch for 3.0.x on 3.0.2 at the earliest?

@tisonkun tisonkun merged commit 56ce296 into apache:master Jun 16, 2023
45 checks passed
@Technoboy-
Copy link
Contributor

I see the mentioned issue #16955 with broker-2.10, so we need at least cherry-pick this to 2.10.

nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request Jul 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Failed to start standalone pulsar service with tiered storage due to class not found problem
8 participants