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

ZOOKEEPER-2574: PurgeTxnLog can inadvertently delete required txn log files #111

Closed
wants to merge 1 commit into from

Conversation

abhishekrai
Copy link

… files

This fix includes patch from Ed Rowe for ZOOKEEPER-2420, which is the same
issue as ZOOKEEPER-2574.

@lvfangmin
Copy link
Contributor

minor comments, others look good to me.

file a snapshot is started and a new transaction log
file is created. The default snapCount is
file a snapshot is started. It also influences rollover
of the current transaction log to a new file. However,
Copy link
Contributor

Choose a reason for hiding this comment

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

keep indention?

Copy link
Author

Choose a reason for hiding this comment

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

Done, thanks.

"\t"+f.getPath());
"\t"+f.getPath();
LOG.info(msg);
System.out.println(msg);
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need to keep both system.out and log4j logging?

Copy link
Author

Choose a reason for hiding this comment

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

It's not ideal but they both serve a purpose that the other cannot as far as I can tell.

System.out.println is useful when this is invoked directly through CLI. The old behavior was to provide just this.
LOG.info is useful in that it's visible in the server log. The old behavior did not log this info which makes debugging through server logs harder.

… files

This fix includes patch from Ed Rowe for ZOOKEEPER-2420, which is the same
issue as ZOOKEEPER-2574.
@abhishekrai
Copy link
Author

Thanks for the review, I've updated the commit.

@asfgit asfgit closed this in 762f4af Jan 23, 2017
asfgit pushed a commit that referenced this pull request Jan 23, 2017
… files

… files

This fix includes patch from Ed Rowe for ZOOKEEPER-2420, which is the same
issue as ZOOKEEPER-2574.

Author: Abhishek Rai <abhishek@thoughtspot.com>

Reviewers: Rakesh Radhakrishnan <rakeshr@apache.org>, Arshad Mohammad <mohammad.arshad@huawei.com>, Abraham Fine <abe@abrahamfine.com>, Allan Lyu <lvfangmin@gmail.com>, Michael Han <hanm@apache.org>

Closes #111 from abhishekrai/ZOOKEEPER-2574

(cherry picked from commit 762f4af)
Signed-off-by: Michael Han <hanm@apache.org>
asfgit pushed a commit that referenced this pull request Jan 23, 2017
… files.

This fix includes patch from Ed Rowe for ZOOKEEPER-2420, which is the same
issue as ZOOKEEPER-2574.
Author: Abhishek Rai <abhishek@thoughtspot.com>

Reviewers: Rakesh Radhakrishnan <rakeshr@apache.org>, Arshad Mohammad <mohammad.arshad@huawei.com>, Abraham Fine <abe@abrahamfine.com>, Allan Lyu <lvfangmin@gmail.com>, Michael Han <hanm@apache.org>

Closes #111 from abhishekrai/ZOOKEEPER-2574
@hanm
Copy link
Contributor

hanm commented Jan 23, 2017

Merged to 3.4, 3.5, and master. Thanks for your contribution @abhishekrai !

@abhishekrai
Copy link
Author

Thanks @hanm!

asfgit pushed a commit that referenced this pull request Nov 15, 2017
ZOOKEEPER-2690: Update documentation source for ZOOKEEPER-2574

Added the documentation changes from PR #111 to the source (zookeeperAdmin.xml) and generated the new version of the html and pdf documents.

Note: I have not updated the 2nd paragraph from ZOOKEEPER-2574 as change "ZOOKEEPER-2349: Update documentation for snapCount" has a more recent version of that part of the text. (ZOOKEEPER-2349 was committed on Sep 11, 2017 while ZOOKEEPER-2574 was committed on Jan 23, 2017.)

Author: Mark Fenes <mfenes@cloudera.com>

Reviewers: phunt@apache.org

Closes #405 from mfenes/ZOOKEEPER-2690_3.4 and squashes the following commits:

a7678f5 [Mark Fenes] ZOOKEEPER-2690: Update documentation source for ZOOKEEPER-2574
ada588b [Mark Fenes] ZOOKEEPER-2690: Update documentation source for ZOOKEEPER-2574

Change-Id: I3f1f582bf9cb720d18f6bd59467f3e5d08214be9
asfgit pushed a commit that referenced this pull request Nov 15, 2017
ZOOKEEPER-2690: Update documentation source for ZOOKEEPER-2574

Added the documentation changes from PR #111 to the source (zookeeperAdmin.xml) and generated the new version of the html and pdf documents.

Note: I have not updated the 2nd paragraph from ZOOKEEPER-2574 as change "ZOOKEEPER-2349: Update documentation for snapCount" has a more recent version of that part of the text. (ZOOKEEPER-2349 was committed on Sep 11, 2017 while ZOOKEEPER-2574 was committed on Jan 23, 2017.)

Author: Mark Fenes <mfenes@cloudera.com>

Reviewers: phunt@apache.org

Closes #404 from mfenes/ZOOKEEPER-2690 and squashes the following commits:

87c5356 [Mark Fenes] ZOOKEEPER-2690: Update documentation source for ZOOKEEPER-2574
0cb3219 [Mark Fenes] ZOOKEEPER-2690: Update documentation source for ZOOKEEPER-2574

Change-Id: I5f0d19007fc97ea488d274cb6ba21c0021aef43e
asfgit pushed a commit that referenced this pull request Nov 15, 2017
ZOOKEEPER-2690: Update documentation source for ZOOKEEPER-2574

Added the documentation changes from PR #111 to the source (zookeeperAdmin.xml) and generated the new version of the html and pdf documents.

Note: I have not updated the 2nd paragraph from ZOOKEEPER-2574 as change "ZOOKEEPER-2349: Update documentation for snapCount" has a more recent version of that part of the text. (ZOOKEEPER-2349 was committed on Sep 11, 2017 while ZOOKEEPER-2574 was committed on Jan 23, 2017.)

Author: Mark Fenes <mfenes@cloudera.com>

Reviewers: phunt@apache.org

Closes #406 from mfenes/ZOOKEEPER-2690_master and squashes the following commits:

ba93365 [Mark Fenes] ZOOKEEPER-2690: Update documentation source for ZOOKEEPER-2574
b152f04 [Mark Fenes] ZOOKEEPER-2690: Update documentation source for ZOOKEEPER-2574

Change-Id: Ibfda946d7b25230308086969011fb2ecb10128d2
lvfangmin pushed a commit to lvfangmin/zookeeper that referenced this pull request Jun 17, 2018
… files

… files

This fix includes patch from Ed Rowe for ZOOKEEPER-2420, which is the same
issue as ZOOKEEPER-2574.

Author: Abhishek Rai <abhishek@thoughtspot.com>

Reviewers: Rakesh Radhakrishnan <rakeshr@apache.org>, Arshad Mohammad <mohammad.arshad@huawei.com>, Abraham Fine <abe@abrahamfine.com>, Allan Lyu <lvfangmin@gmail.com>, Michael Han <hanm@apache.org>

Closes apache#111 from abhishekrai/ZOOKEEPER-2574
lvfangmin pushed a commit to lvfangmin/zookeeper that referenced this pull request Jun 17, 2018
ZOOKEEPER-2690: Update documentation source for ZOOKEEPER-2574

Added the documentation changes from PR apache#111 to the source (zookeeperAdmin.xml) and generated the new version of the html and pdf documents.

Note: I have not updated the 2nd paragraph from ZOOKEEPER-2574 as change "ZOOKEEPER-2349: Update documentation for snapCount" has a more recent version of that part of the text. (ZOOKEEPER-2349 was committed on Sep 11, 2017 while ZOOKEEPER-2574 was committed on Jan 23, 2017.)

Author: Mark Fenes <mfenes@cloudera.com>

Reviewers: phunt@apache.org

Closes apache#406 from mfenes/ZOOKEEPER-2690_master and squashes the following commits:

ba93365 [Mark Fenes] ZOOKEEPER-2690: Update documentation source for ZOOKEEPER-2574
b152f04 [Mark Fenes] ZOOKEEPER-2690: Update documentation source for ZOOKEEPER-2574

Change-Id: Ibfda946d7b25230308086969011fb2ecb10128d2
RokLenarcic pushed a commit to RokLenarcic/zookeeper that referenced this pull request Sep 3, 2022
… files

… files

This fix includes patch from Ed Rowe for ZOOKEEPER-2420, which is the same
issue as ZOOKEEPER-2574.

Author: Abhishek Rai <abhishek@thoughtspot.com>

Reviewers: Rakesh Radhakrishnan <rakeshr@apache.org>, Arshad Mohammad <mohammad.arshad@huawei.com>, Abraham Fine <abe@abrahamfine.com>, Allan Lyu <lvfangmin@gmail.com>, Michael Han <hanm@apache.org>

Closes apache#111 from abhishekrai/ZOOKEEPER-2574
RokLenarcic pushed a commit to RokLenarcic/zookeeper that referenced this pull request Sep 3, 2022
ZOOKEEPER-2690: Update documentation source for ZOOKEEPER-2574

Added the documentation changes from PR apache#111 to the source (zookeeperAdmin.xml) and generated the new version of the html and pdf documents.

Note: I have not updated the 2nd paragraph from ZOOKEEPER-2574 as change "ZOOKEEPER-2349: Update documentation for snapCount" has a more recent version of that part of the text. (ZOOKEEPER-2349 was committed on Sep 11, 2017 while ZOOKEEPER-2574 was committed on Jan 23, 2017.)

Author: Mark Fenes <mfenes@cloudera.com>

Reviewers: phunt@apache.org

Closes apache#406 from mfenes/ZOOKEEPER-2690_master and squashes the following commits:

ba93365 [Mark Fenes] ZOOKEEPER-2690: Update documentation source for ZOOKEEPER-2574
b152f04 [Mark Fenes] ZOOKEEPER-2690: Update documentation source for ZOOKEEPER-2574

Change-Id: Ibfda946d7b25230308086969011fb2ecb10128d2
PapaCharlie pushed a commit to PapaCharlie/zookeeper that referenced this pull request Jun 9, 2023
ZkClientUriDomainMappingHelper instance is registered as a watcher during the instantiation process, so it will get the notification every time there's a change in the client URI domain mapping, and it can update each session that are connected to this zookeeper server for the updated access control metatdata.
However, in ZooKeeper server code, there is an implicit assumption that the objects registered as a watcher are ServerCnxn objects, while ZkClientUriDomainMappingHelper is not an instance of ServerCnxn. This incorrect cast of ZkClientUriDomainMappingHelper to ServerCnxn can lead to code failure.
This commit leveraged a dummy server cnxn class `DumbWatcher` which actually functions as a watcher, to prevent this error.
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.

3 participants