Thanks for the update Sivabalan. I will wait for your fix.

On Tue, Mar 10, 2020 at 12:36 PM Sivabalan <[email protected]> wrote:

> thanks for bringing this to my attention Prasant. Yes, I bumped into the
> bug couple of days back. I am working on the fix, and the expected no of
> records might have to be fixed as well. I am running into issues debugging
> continuous tests as of now. But I am working on it.
>
>
> On Tue, Mar 10, 2020 at 12:32 PM Prashant Wason <[email protected]>
> wrote:
>
> > Hi Team,
> >
> > While exploring HUDI source code I came across this PR:
> >
> https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_apache_incubator-2Dhudi_pull_1073&d=DwIBaQ&c=r2dcLCtU9q6n0vrtnDw9vg&r=c89AU9T1AVhM4r2Xi3ctZA&m=9WZ2tqIxWwOrZRAqmP_InSRBlFhGKElcWnFP-DPgCkY&s=s8ZOjL4LXWaB6kfrL-BUZdOwb22h4RA4ff9KdUrfTNk&e=
> >
> > As part of the above PR, generation of delete records was added
> > to HoodieTestDataGenerator. Within the class HoodieTestDataGenerator, the
> > existingKeys Map maintains the current keys. In the above PR, the
> following
> > code was added to delete from the Map:
> >
> > existingKeys.remove(kp);
> >
> > This is delete by value rather than the key (private final Map<Integer,
> > KeyPartition> existingKeys;)
> >
> > I tried fixing this issue but this leads to unit test failures
> > in TestHoodieDeltaStreamer within the testUpsertsCOWContinuousMode. The
> > code which is failing is this check (bold):
> >
> >     TestHelpers.waitTillCondition((r) -> {
> >       if (tableType.equals(HoodieTableType.MERGE_ON_READ)) {
> >         TestHelpers.assertAtleastNDeltaCommits(5, tableBasePath, dfs);
> >         TestHelpers.assertAtleastNCompactionCommits(2, tableBasePath,
> dfs);
> >       } else {
> >         TestHelpers.assertAtleastNCompactionCommits(5, tableBasePath,
> dfs);
> >       }
> >       *TestHelpers.assertRecordCount(totalRecords + 200, tableBasePath +
> > "/*/*.parquet", sqlContext);*
> >       *TestHelpers.assertDistanceCount(totalRecords + 200, tableBasePath
> +
> > "/*/*.parquet", sqlContext);*
> >       return true;
> >
> > I did not understand why a +200 was added in the checks above? Is this
> > related to the existingKeys.remove() which does not remove the records
> from
> > the Map?
> >
> > I have left these comments on the PR itself so they are easier to read.
> >
> > Thanks
> > Prashant
> >
>
>
> --
> Regards,
> -Sivabalan
>

Reply via email to