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

HDDS-4468. Fix Goofys listBucket large than 1000 objects will stuck forever #1595

Merged
merged 5 commits into from
Nov 23, 2020

Conversation

maobaolong
Copy link
Member

@maobaolong maobaolong commented Nov 16, 2020

Description

The following is the ListObjectsOutput, ListObjectsV2Output and ListObjectsInput, ListObjectsV2Input from github.com/aws/aws-sdk-go/service/s3/api.go,

For goofys talk to s3g, it use the ListObjectsOutput and ListObjectsInput, so I think s3g should return NextMarker in the ListBucketResult and use mark to list key from.

For this PR, I purpose to make BucketEndpoint of s3g can both serve to V1 and V2

type ListObjectsOutput struct {
	_ struct{} `type:"structure"`
	CommonPrefixes []*CommonPrefix `type:"list" flattened:"true"`
	Contents []*Object `type:"list" flattened:"true"`
	Delimiter *string `type:"string"`
	EncodingType *string `type:"string" enum:"EncodingType"`
	IsTruncated *bool `type:"boolean"`
	Marker *string `type:"string"`
	MaxKeys *int64 `type:"integer"`
	Name *string `type:"string"`
	NextMarker *string `type:"string"`
	Prefix *string `type:"string"`
}

type ListObjectsV2Output struct {
	_ struct{} `type:"structure"`
	CommonPrefixes []*CommonPrefix `type:"list" flattened:"true"`
	Contents []*Object `type:"list" flattened:"true"`
	ContinuationToken *string `type:"string"`
	Delimiter *string `type:"string"`
	EncodingType *string `type:"string" enum:"EncodingType"`
	IsTruncated *bool `type:"boolean"`
	KeyCount *int64 `type:"integer"`
	MaxKeys *int64 `type:"integer"`
	Name *string `type:"string"`
	NextContinuationToken *string `type:"string"`
	Prefix *string `type:"string"`
	StartAfter *string `type:"string"`
}

type ListObjectsInput struct {
	_ struct{} `type:"structure"`
	Bucket *string `location:"uri" locationName:"Bucket" type:"string" required:"true"`
	Delimiter *string `location:"querystring" locationName:"delimiter" type:"string"`
	EncodingType *string `location:"querystring" locationName:"encoding-type" type:"string" enum:"EncodingType"`
	Marker *string `location:"querystring" locationName:"marker" type:"string"`
	MaxKeys *int64 `location:"querystring" locationName:"max-keys" type:"integer"`
	Prefix *string `location:"querystring" locationName:"prefix" type:"string"`
	RequestPayer *string `location:"header" locationName:"x-amz-request-payer" type:"string" enum:"RequestPayer"`
}

type ListObjectsV2Input struct {
	_ struct{} `type:"structure"`

	Bucket *string `location:"uri" locationName:"Bucket" type:"string" required:"true"`
	ContinuationToken *string `location:"querystring" locationName:"continuation-token" type:"string"`
	Delimiter *string `location:"querystring" locationName:"delimiter" type:"string"`
	EncodingType *string `location:"querystring" locationName:"encoding-type" type:"string" enum:"EncodingType"`
	FetchOwner *bool `location:"querystring" locationName:"fetch-owner" type:"boolean"`
	MaxKeys *int64 `location:"querystring" locationName:"max-keys" type:"integer"`
	Prefix *string `location:"querystring" locationName:"prefix" type:"string"`
	RequestPayer *string `location:"header" locationName:"x-amz-request-payer" type:"string" enum:"RequestPayer"`
	StartAfter *string `location:"querystring" locationName:"start-after" type:"string"`
}

What changes were proposed in this pull request?

Fill startAfter with marker parameter when startAfter is null

Fill nextMark to the response, so that Goofys can use the nextMark to list follow-up objects.

What is the link to the Apache JIRA

HDDS-4468

How was this patch tested?

  • Create an ozone cluster with s3g.
  • Start a goofys with this s3g.
  • Generate more than 1000 file in the test bucket
  • list the goofys mounted path.

I've tested this PR on my local env.

@maobaolong maobaolong closed this Nov 16, 2020
@maobaolong maobaolong reopened this Nov 16, 2020
Copy link
Contributor

@runzhiwang runzhiwang left a comment

Choose a reason for hiding this comment

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

LGTM.

@maobaolong
Copy link
Member Author

@adoroszlai Would you please take a look at this PR, thank you.

@maobaolong
Copy link
Member Author

@runzhiwang Thanks for your quick review, I push a new commit to add some necessary comments, PTAL.

@maobaolong maobaolong closed this Nov 17, 2020
@maobaolong maobaolong reopened this Nov 17, 2020
@maobaolong
Copy link
Member Author

@GlenGeng Thanks for your review, I push a new commit to update the comment, PTAL.

@GlenGeng-awx
Copy link
Contributor

+1 LGTM

Copy link
Contributor

@timmylicheng timmylicheng left a comment

Choose a reason for hiding this comment

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

LGTM. +1

Copy link
Member

@captainzmc captainzmc left a comment

Choose a reason for hiding this comment

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

LGTM. +1

Copy link
Contributor

@bharatviswa504 bharatviswa504 left a comment

Choose a reason for hiding this comment

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

I have one comment.

@maobaolong
Copy link
Member Author

@bharatviswa504 Thanks for your review and suggestion, I've set the marker to response also when marker is not null.

PTAL.

@maobaolong maobaolong closed this Nov 23, 2020
@maobaolong maobaolong reopened this Nov 23, 2020
@maobaolong maobaolong closed this Nov 23, 2020
@maobaolong maobaolong reopened this Nov 23, 2020
@maobaolong maobaolong closed this Nov 23, 2020
@maobaolong maobaolong reopened this Nov 23, 2020
Copy link
Member

@elek elek left a comment

Choose a reason for hiding this comment

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

+1 thanks the fix @maobaolong

@maobaolong maobaolong closed this Nov 23, 2020
@maobaolong maobaolong reopened this Nov 23, 2020
@maobaolong maobaolong closed this Nov 23, 2020
@maobaolong maobaolong reopened this Nov 23, 2020
Copy link
Contributor

@bharatviswa504 bharatviswa504 left a comment

Choose a reason for hiding this comment

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

+1 LGTM

@bharatviswa504 bharatviswa504 merged commit 6cc4a43 into apache:master Nov 23, 2020
@bharatviswa504
Copy link
Contributor

Thank You everyone for the review and @maobaolong for the contribution.

errose28 added a commit to errose28/ozone that referenced this pull request Nov 24, 2020
* HDDS-3698-upgrade: (46 commits)
  HDDS-4468. Fix Goofys listBucket large than 1000 objects will stuck forever (apache#1595)
  HDDS-4417. Simplify Ozone client code with configuration object -- addendum (apache#1581)
  HDDS-4476. Improve the ZH translation of the HA.md in doc. (apache#1597)
  HDDS-4432. Update Ratis version to latest snapshot. (apache#1586)
  HDDS-4488. Open RocksDB read only when loading containers at Datanode startup (apache#1605)
  HDDS-4478. Large deletedKeyset slows down OM via listStatus. (apache#1598)
  HDDS-4452. findbugs.sh couldn't be executed after a full build (apache#1576)
  HDDS-4427. Avoid ContainerCache in ContainerReader at Datanode startup (apache#1549)
  HDDS-4448. Duplicate refreshPipeline in listStatus (apache#1569)
  HDDS-4450. Cannot run ozone if HADOOP_HOME points to Hadoop install (apache#1572)
  HDDS-4346.Ozone specific Trash Policy (apache#1535)
  HDDS-4426. SCM should create transactions using all blocks received from OM (apache#1561)
  HDDS-4399. Safe mode rule for piplelines should only consider open pipelines. (apache#1526)
  HDDS-4367. Configuration for deletion service intervals should be different for OM, SCM and datanodes (apache#1573)
  HDDS-4462. Add --frozen-lockfile to pnpm install to prevent ozone-recon-web/pnpm-lock.yaml from being updated automatically (apache#1589)
  HDDS-4082. Create ZH translation of HA.md in doc. (apache#1591)
  HDDS-4464. Upgrade httpclient version due to CVE-2020-13956. (apache#1590)
  HDDS-4467. Acceptance test fails due to new Hadoop 3 image (apache#1594)
  HDDS-4466. Update url in .asf.yaml to use TLP project (apache#1592)
  HDDS-4458. Fix Max Transaction ID value in OM. (apache#1585)
  ...
errose28 added a commit to errose28/ozone that referenced this pull request Nov 25, 2020
* HDDS-3698-upgrade: (47 commits)
  HDDS-4468. Fix Goofys listBucket large than 1000 objects will stuck forever (apache#1595)
  HDDS-4417. Simplify Ozone client code with configuration object -- addendum (apache#1581)
  HDDS-4476. Improve the ZH translation of the HA.md in doc. (apache#1597)
  HDDS-4432. Update Ratis version to latest snapshot. (apache#1586)
  HDDS-4488. Open RocksDB read only when loading containers at Datanode startup (apache#1605)
  HDDS-4478. Large deletedKeyset slows down OM via listStatus. (apache#1598)
  HDDS-4452. findbugs.sh couldn't be executed after a full build (apache#1576)
  HDDS-4427. Avoid ContainerCache in ContainerReader at Datanode startup (apache#1549)
  HDDS-4448. Duplicate refreshPipeline in listStatus (apache#1569)
  HDDS-4450. Cannot run ozone if HADOOP_HOME points to Hadoop install (apache#1572)
  HDDS-4346.Ozone specific Trash Policy (apache#1535)
  HDDS-4426. SCM should create transactions using all blocks received from OM (apache#1561)
  HDDS-4399. Safe mode rule for piplelines should only consider open pipelines. (apache#1526)
  HDDS-4367. Configuration for deletion service intervals should be different for OM, SCM and datanodes (apache#1573)
  HDDS-4462. Add --frozen-lockfile to pnpm install to prevent ozone-recon-web/pnpm-lock.yaml from being updated automatically (apache#1589)
  HDDS-4082. Create ZH translation of HA.md in doc. (apache#1591)
  HDDS-4464. Upgrade httpclient version due to CVE-2020-13956. (apache#1590)
  HDDS-4467. Acceptance test fails due to new Hadoop 3 image (apache#1594)
  HDDS-4466. Update url in .asf.yaml to use TLP project (apache#1592)
  HDDS-4458. Fix Max Transaction ID value in OM. (apache#1585)
  ...
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.

7 participants