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

While appending, use the same generation stamp sent by the namenode #51

Closed
wants to merge 1 commit into from

Conversation

muralirvce
Copy link

While trying out the append functionality ran into the issue where the datanode was complaining about generation stamp(gs) mismatch, and looks like gs must be same as sent by namenode. After changing this way the append works fine.

@colinmarc
Copy link
Owner

Hi @muralirvce, thanks for the fix. Sorry about the delay responding!

Are we missing a test for this behavior, and if so would it be possible to add one?

@muralirvce
Copy link
Author

Hi Colin,
The second append to the same file fails . Existing append test must
test this out .

Thanks,
Murali

On Thursday, September 22, 2016, Colin Marc notifications@github.com
wrote:

Hi @muralirvce https://github.com/muralirvce, thanks for the fix. Sorry
about the delay responding!

Are we missing a test for this behavior, and if so would it be possible to
add one?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#51 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ALsMY7_Dtlko3Q21pu_RY5RbA1wlqLF6ks5qsxhAgaJpZM4J5p4L
.

MURALI

@colinmarc
Copy link
Owner

@muralirvce it seems like there's already a test for that, no? If you don't mind adding a test to the PR (that fails without this change), that would be great.

https://github.com/colinmarc/hdfs/blob/master/file_writer_test.go#L263-L306

@colinmarc
Copy link
Owner

Ah, I see what you mean. I opened #53, which has a test, and I'm reaching out to the person who contributed the append implementation for confirmation.

Thanks for the report!

@colinmarc colinmarc closed this Sep 23, 2016
@muralirvce
Copy link
Author

Sure I can add the test.

@colinmarc
Copy link
Owner

No need! I already added it in #53.

@muralirvce
Copy link
Author

Thanks Colin. If you can pull the fix, I shall also incorporate this in my
production build. Thanks for the help!

On Fri, Sep 23, 2016 at 8:30 AM, Colin Marc notifications@github.com
wrote:

No need! I already added it in #53
#53.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#51 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ALsMY9t2vfljamzgBdy6itotUQr-ALebks5qs_CNgaJpZM4J5p4L
.

MURALI

@colinmarc
Copy link
Owner

Done!

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.

None yet

2 participants