Fix for tracing context in sub-sequential and parallel operations#95
Fix for tracing context in sub-sequential and parallel operations#95anujmodi2021 merged 11 commits intoABFS_3.3.2_devfrom
Conversation
saxenapranav
left a comment
saxenapranav
left a comment
There was a problem hiding this comment.
Good with code. Have suggestions around test. Thanks!
| import static org.apache.hadoop.fs.azurebfs.services.AbfsClientTestUtil.addMockBehaviourToAbfsClient; | ||
| import static org.apache.hadoop.fs.azurebfs.services.AbfsClientTestUtil.addMockBehaviourToRestOpAndHttpOp; |
There was a problem hiding this comment.
seems no new code is added here. we can remove the imports.
There was a problem hiding this comment.
These methods were moved to a utils file from where they can be used by other tests as well...
...ls/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/AbfsClientTestUtil.java
Show resolved
Hide resolved
anujmodi2021
force-pushed
the
listTracingContextFix
branch
from
2adab61 to
6c2781b
Compare
anujmodi2021
changed the title
saxenapranav
dismissed
their stale review
Suggestion has been taken and resolved.
| list.setNextMarker("nextMarker"); | ||
| } | ||
|
|
||
| public static void hookOnRestOpsForTracingContextSingularity(AbfsClient client) { |
There was a problem hiding this comment.
Can we also validate that for sub ops the primary request id is same only the object is changing ? Just a minor suggestion
There was a problem hiding this comment.
This test has been added as part of https://github.com/ABFSDriver/AbfsHadoop/pull/76/files.
There was a problem hiding this comment.
Though apart from this, in any test in which tracingHeaderValidator is created, this shall be checked.
How?
the tc created by fs -> puts primaryRequestId in tracingHeaderValidator.
And the new TracingContext() doesnt change the state of tracingHeaderValidator field values (though clones it).
So any newTracingContext done will have the tracingHeaderValidator having values populated by original tc.
| op.setResult(result); | ||
| } | ||
|
|
||
| public static void assertTracingContextInstantiationCount(final AbfsClient client, final int numberOfInstantiation) { |
There was a problem hiding this comment.
This method is not used anywhere ?
There was a problem hiding this comment.
Yes,
It is not used anywhere...
Removing it.
...ls/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/AbfsClientTestUtil.java
Outdated
Show resolved
Hide resolved
4 participants
No description provided.